support optional start elements in xmlstreamwriter. #1193
Conversation
this adds methods to xmlstreamwriter to allow conditional writing of a start element. The conditional element will only be written to the stream if it has children, which is deteremined when the condtional element is ended. This simplifies writing of such elements as the writer does not need to know if it will subsequently write children or not.
|
I think this fixes one or more bugs related to the current necessity to check to see if we have data for every possible extension element we might want to write before we write the extensions element. Then, subsequently we also have to check if we have data for each extension element to see if we should write it. Even if there isn't a bug that dual checking is hard to maintain. |
|
@robertlipe this is groundwork for my desire to move garmin_fs_xml_fprint into GpxFormat::gpx_write_common_extensions, and only write the garmin extensions when the gpx option garminextensions is enabled. This will simplify the decision tree in GpxFormat::gpx_waypt_pr. Today we either do passthrough, or print one subset of the garmin extensions(garmin_fs_xml_fprint), or print a different subset of the garmin extensions(gpx_write_common_extensions) based on the existence of gmsd data, and the value of the garminextensions and humminbirdextensions options. For the reasons in my preceding comment I think this PR is desirable by itself, but it is also blocking my desires explained in this comment. Are you ok with this PR? You can review my potential consolidation in an upcoming PR if this PR is accepted. |
|
Agreed all around. |
|
I don't see a scalability problem with the number of wpts/rtepts/trkpts. As soon as the startOptionalElement is paired with a endElement that stack is destructed. In our usage all stack usage is contained is bounded by an extensions element. And I would argue the extensions element content is finite. |
|
Sorry. I meant Scalability in the number of special cases inside the code
as the number of tags grows, not anything with the count of things
involved.
…On Fri, Oct 27, 2023, 3:09 PM tsteven4 ***@***.***> wrote:
Merged #1193 <#1193> into master.
—
Reply to this email directly, view it on GitHub
<#1193 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3VAD7QR5CAEA7HVXFZWRTYBQIARAVCNFSM6AAAAAA6MRJ726VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJQG44TKNBSGY4DMNA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
See #1200. I think it is easier to understand when we write what for waypoints, it is now simply controlled by the gpx writer extension options. Any unrecognized tags go through tt_unknown on read. This does mean we have to catch all tags from the gpx namespace so they don't go through tt_unknown. We added some missing ones in #1191. This set is finite, but if new elements are added to gpx 1.2 they could cause problems. We might be able to guard against this by checking the namespace. It would be easy to handle this in gpx 1.1 because all the foreign stuff is within gpx extension tags. But gpx 1.0 isn't so kind. |
|
Yep, that's where I knēw we were going. I like it.
It basically makes us accept the reality that there are no unknown tags in
GPX for us; we have to know about them all.
I have wondered if our trick of recognizing "gpxx:" as associating as the
XMlns for an extension is just wrong. Someone that's basically an
indirection point, isn't it, where we should booking up gpxx and confirming
that it's bound to Garmin.com/... Blah.xsd., shouldn't we?
If that's the case,. It's interesting that we've had as little. Diversity
in our input as. I'd expect.
…On Fri, Oct 27, 2023, 4:28 PM tsteven4 ***@***.***> wrote:
See #1200 <#1200>.
I think it is easier to understand when we write what for waypoints, it is
now simply controlled by the gpx writer extension options.
Any unrecognized tags go through tt_unknown on read. This does mean we
have to catch all tags from the gpx namespace so they don't go through
tt_unknown. We added some missing ones in #1191
<#1191>. This set is finite, but
if new elements are added to gpx 1.2 they could cause problems. We might be
able to guard against this by checking the namespace. It would be easy to
handle this in gpx 1.1 because all the foreign stuff is within gpx
extension tags. But gpx 1.0 isn't so kind.
—
Reply to this email directly, view it on GitHub
<#1193 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD37DKKVWZU6EZFRUOADYBQRO7AVCNFSM6AAAAAA6MRJ726VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGUZTCMBRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Your correct, we probably still take some liberties assuming prefixes are associated with the expected namespaces. On gpx read we do attempt to map some namespaces to the expected prefix used in the tags (GpxFormat::qualifiedName()). I also suspect if we had a gpx file where the gpx namespace used a prefix we would crash and burn. I have been considering using more of the namespace support in QXmlStreamWriter. |
this adds methods to xmlstreamwriter to allow conditional writing of a start element. The conditional element will only be written to the stream if it has children, which is deteremined when the condtional element is ended. This simplifies writing of such elements as the writer does not need to know if it will subsequently write children or not.
This is groundwork for further possible improvements in the gpx writer extension handling. For example, we merge garmin_fs_xml_fprint and gpx_write_common_extensions, only writing the extensions when gpx garminextensions option is selected? Another example is the use of TrackPointExtension v2 instead of v1, allowing speed and course to be written in gpx 1.1. Shouldn't we make the default gpx write version 1.1, it has been out since 8/2004!