search

GPSBabel / gpsbabel

GPSBabel: convert, manipulate, and transfer data from GPS programs or GPS receivers. Open Source and supported on MacOS, Windows, Linux, and more. Pointy clicky GUI or a command line version...
https://www.gpsbabel.org
GNU General Public License v2.0
470 stars 125 forks source link

refactor garmin_txt date time handling #1208

Open tsteven4 opened 9 months ago

tsteven4 commented 9 months ago

This is an alternative to #1205.

Instead of using QDateTime format strings directly, we translate our own "human" format strings to QDateTime format strings. This means there isn't a user interface change for garmin_txt which isn't important.

If we extend this to retire strptime/strpftime, we may want a user interface change there from strptime/strftime format strings to our "human" strings. I would argue that would be a better direction than attempting to translate the more complex strptime/strftime strings, and I think our next release is 2.0.0 which should give us some liberty to change the user interface. Our human formats offer more limited functionality than strptime/strftime, making it easier to change the underlying implementation as this PR does. It would also be consistent across formats & filters.

Neither our human format strings or strptime/strftime format strings support fractional seconds. To cover the existing use cases of strptime/strptime this isn't necessary. Qt's implementation for fractional seconds with user supplied format strings is problematic with fractional seconds as was discussed in #1205.

Escaping a single quote in a QDateTime::fromString format string is under-documented, and different from QDateTime::toString! The details are in convert_human_date_format and convert_human_time_format.

tsteven4 commented 9 months ago

@robertlipe do you like this direction better than #1205?

I skipped your suggested renames for now to minimize the diffs.

tsteven4 commented 9 months ago

One problem with generalizing our human date and time formats is that they both use '[mM]' for different things (month or minute), which makes supporting a human datetime format as xcsv sometimes uses, e.g. GMT_TIME, problematic. garmin_txt assumes date always precedes time, so if that is general enough for xcsv it is a way out.

tsteven4 commented 9 months ago

so if that is general enough for xcsv it is a way out.

not really. GMT_TIME has one format string and it isn't clear we could break it in pieces.

GPSBabelDeveloper commented 9 months ago

That's a more shallow cut that delivers most (all) of the results. It's hard to argue with results. I'm quite OK with this. It keeps us out of lots of unanticipated trouble.

Add a static_assert() in main or something that ensures the escaping remains the same.

Maybe strftime_to_timespec() was just a bad idea, trying to solve a problem better fixed at the source.

Go for it.

On Thu, Nov 2, 2023 at 4:44 PM tsteven4 @.***> wrote:

so if that is general enough for xcsv it is a way out.

not really. GMT_TIME has one format string and it isn't clear we could break it in pieces.

— Reply to this email directly, view it on GitHub https://github.com/GPSBabel/gpsbabel/pull/1208#issuecomment-1791581152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3VAD7UD3LV7S37I3HRMP3YCQH5TAVCNFSM6AAAAAA63PMIVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJRGU4DCMJVGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

tsteven4 commented 9 months ago

I intend to make the human formats case sensitive, which allows distinguishing minutes and Months as in ISO8601, Qt:DateTime, etc.