simplify formatting of date/time in garmin_txt #1205
Conversation
and leave memory management up to the copy on write container.
this prevents the possiblity of detachment of the ilinks list.
to implement the garmin_txt utc option.
|
@robertlipe before I explore expanding this idea to replace strptime/strftime I'd like to get your opinion on this approach. |
There was a problem hiding this comment.
As you note, I'm not sure why 'garmin_txt' exists. I thought we killed this before. I think it's like a copy/paste of some ancient mapsource screen or something that's not actually read or written by anything, but I could be wrong. (We definitely had something like that in our past, though.)
You're right that on their own, convert_human_foo and strftime_to_foo look pretty ridiculous. (It also looks like the authors of each should talk to the other...which is pretty sad, because I'm pretty sure they're both me. :-) ) My master plan was to internally replace strftime/strptime and friends with QDateTime formatters, but keep the stuff visible in the style files as C89 specifiers for compatibility. My overkill kicked in and I thought about adding versioning to the style files and converting (or not) based on a found version number/token so we could preserve any (likely non-existent) style files that aren't checked in. Thus I turned a 20 minute search and replace into a deadlocked mess. Sorry.
A version number "2.0" would be a good time to rip off that band-aid, fix the style files, and just say that QDateTime specifiers are what we use, though we might be painting ourselves into QDateTime corner instead of a C89 corner when there's a third or even fourth unpainted corner in the room, std::chrono and our (currently short-lived) friend, std::format, though I think we're pretty locked in to Qt at this point.
Reference: https://en.cppreference.com/w/cpp/chrono/duration/formatter
So for the questions that I think you did ask, and some bonus that you didn't...
I agree that two is probably too many. (Three formatting specifier standards is definitely too many.)
-
sscanftime() has very few callers and is visible in only two tokens. addhms() is weird. We have latitude (especially if we're willing to change the style files or subtly change how XT.*TIME works) on changing calling conventions and/or results. Maybe we just change the rules and say that you can only express times as an HMS molecule instead of H, M, and S atoms. Someone with a spreadheet with separate fields like that might be sad, but I think we can justify that change for us to stay out of the token processign business ourselves.
-
Fractional seconds in C89 was one reason we landed up in Qt land. Is the issue that it misparses > 3 digits? so hh:mm:ss.1234 doesn't get that .4Ms? I don't think that makes me sad. Milliseconds is a very reasonable bottom floor when parsing ASCII input, IMO.
-
Are you saying the reading and writing pairs must match? That seems natural. Are you saying that mixing Qt-isms and C89-isms in one format string doesn't work? I'd treat that as a given as there are some tokens with different definitions.
Aside: we have extremely wide latitude in what we do with garmin_txt, up to and including vaporizing it. I was just experimenting with the conversion of format strings there as it had a small blast radius if I broke it.
Aside2: I was never really convinced our .ini files were that useful. Do we have evidence people use them? Ditto sessions. In hindsight, I probably let Olaf run a bit too wild at the time... (Notice how many people missed CET?)
We have only 16 style files. I'd wager that way more than half of them could disappear without notice. arc, custom, garmin_poi might be the only ones of those that are really used by end users. The rest are just kind of along because styles were meant to be super light-weight alternatives to writing C. IMO, our unicsv took a lot of that load and there just aren't free text formats in programs like there used to be. openoffice and tabsep exist to interface with other programs, like awk or perl or whatever. I DO think that GSAK uses its own custom style for internal data trading with us, but they're frozen on an ancient GPSBabel build and unlikely to track us anyway. So I suspect we really can quit dancing around the compatibility on those files.
| @@ -190,16 +189,9 @@ get_option_val(const char* option, const char* def) | |||
| static void | |||
| init_date_and_time_format() | |||
There was a problem hiding this comment.
This is a constructor wearing C groucho glasses?
| @@ -364,30 +356,37 @@ print_position(const Waypoint* wpt) | |||
| } | |||
|
|
|||
| static void | |||
| print_date_and_time(const time_t time, const bool time_only) | |||
| print_duration(int time) | |||
There was a problem hiding this comment.
Consider a name for time that hints what it is. 'seconds', I think? print_duration_as_hms() ? I'm not sure.
| @@ -444,7 +443,7 @@ print_distance(const double distance, const bool no_scale, const bool with_tab, | |||
| } | |||
|
|
|||
| static void | |||
| print_speed(const double distance, const time_t time) | |||
| print_speed(const double distance, int time) | |||
There was a problem hiding this comment.
While you're in here, can ouo please do a sarch and replace and make these 'meters' and 'seconds' maybe?
No, the issue is QDateTime::fromString returns an invalid date time. |
|
Gross. So we suppressmtrsiling 0., anything after 3 spaces and reparse.
With s.z on failure?
Is it. More amenable to things that look more like times and have an hours
and minutes component?
…On Thu, Nov 2, 2023, 8:41 AM tsteven4 ***@***.***> wrote:
2. Fractional seconds in C89 was one reason we landed up in Qt land. Is
the issue that it misparses > 3 digits? so hh:mm:ss.1234 doesn't get that
.4Ms? I don't think that makes me sad. Milliseconds is a very reasonable
bottom floor when parsing ASCII input, IMO.
No, the issue is QDateTime::fromString returns an invalid date time.
"0" "s" QDateTime(1900-01-01 00:00:00.000 MST Qt::LocalTime)
"0." "s" QDateTime(Invalid)
"0.0" "s" QDateTime(Invalid)
"0.00" "s" QDateTime(Invalid)
"0.000" "s" QDateTime(Invalid)
"0.0000" "s" QDateTime(Invalid)
"10" "s" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10." "s" QDateTime(Invalid)
"10.0" "s" QDateTime(Invalid)
"10.00" "s" QDateTime(Invalid)
"10.000" "s" QDateTime(Invalid)
"10.0000" "s" QDateTime(Invalid)
"0" "s.z" QDateTime(Invalid)
"0." "s.z" QDateTime(Invalid)
"0.0" "s.z" QDateTime(1900-01-01 00:00:00.000 MST Qt::LocalTime)
"0.00" "s.z" QDateTime(1900-01-01 00:00:00.000 MST Qt::LocalTime)
"0.000" "s.z" QDateTime(1900-01-01 00:00:00.000 MST Qt::LocalTime)
"0.0000" "s.z" QDateTime(Invalid)
"10" "s.z" QDateTime(Invalid)
"10." "s.z" QDateTime(Invalid)
"10.0" "s.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.00" "s.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.000" "s.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.0000" "s.z" QDateTime(Invalid)
"0" "s.zzz" QDateTime(Invalid)
"0." "s.zzz" QDateTime(Invalid)
"0.0" "s.zzz" QDateTime(Invalid)
"0.00" "s.zzz" QDateTime(Invalid)
"0.000" "s.zzz" QDateTime(1900-01-01 00:00:00.000 MST Qt::LocalTime)
"0.0000" "s.zzz" QDateTime(Invalid)
"10" "s.zzz" QDateTime(Invalid)
"10." "s.zzz" QDateTime(Invalid)
"10.0" "s.zzz" QDateTime(Invalid)
"10.00" "s.zzz" QDateTime(Invalid)
"10.000" "s.zzz" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.0000" "s.zzz" QDateTime(Invalid)
"0" "ss" QDateTime(Invalid)
"0." "ss" QDateTime(Invalid)
"0.0" "ss" QDateTime(Invalid)
"0.00" "ss" QDateTime(Invalid)
"0.000" "ss" QDateTime(Invalid)
"0.0000" "ss" QDateTime(Invalid)
"10" "ss" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10." "ss" QDateTime(Invalid)
"10.0" "ss" QDateTime(Invalid)
"10.00" "ss" QDateTime(Invalid)
"10.000" "ss" QDateTime(Invalid)
"10.0000" "ss" QDateTime(Invalid)
"0" "ss.z" QDateTime(Invalid)
"0." "ss.z" QDateTime(Invalid)
"0.0" "ss.z" QDateTime(Invalid)
"0.00" "ss.z" QDateTime(Invalid)
"0.000" "ss.z" QDateTime(Invalid)
"0.0000" "ss.z" QDateTime(Invalid)
"10" "ss.z" QDateTime(Invalid)
"10." "ss.z" QDateTime(Invalid)
"10.0" "ss.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.00" "ss.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.000" "ss.z" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.0000" "ss.z" QDateTime(Invalid)
"0" "ss.zzz" QDateTime(Invalid)
"0." "ss.zzz" QDateTime(Invalid)
"0.0" "ss.zzz" QDateTime(Invalid)
"0.00" "ss.zzz" QDateTime(Invalid)
"0.000" "ss.zzz" QDateTime(Invalid)
"0.0000" "ss.zzz" QDateTime(Invalid)
"10" "ss.zzz" QDateTime(Invalid)
"10." "ss.zzz" QDateTime(Invalid)
"10.0" "ss.zzz" QDateTime(Invalid)
"10.00" "ss.zzz" QDateTime(Invalid)
"10.000" "ss.zzz" QDateTime(1900-01-01 00:00:10.000 MST Qt::LocalTime)
"10.0000" "ss.zzz" QDateTime(Invalid)
—
Reply to this email directly, view it on GitHub
<#1205 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3VAD3LISGWQCNWCE3KLWDYCOPIPAVCNFSM6AAAAAA6ZMC2T6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJQG42TONJTGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
This is a user visible change.
This is a test of a solution to #1113.
Currently garmin_txt readers and writers use a homegrown "human" date and time format. This is converted to a strptime/strftime format in util.cc by convert_human_date_format, convert_human_time_format.
The garmin_txt reader then converts the strptime/strftime format to a QDateTime format, which is remarkably similar to what we started with! The garmin_txt writer uses strftime.
This PR changes the format for date and time to that of QDateTime, eliminating all the conversions and the use of strftime in garmin_txt.
There are some potential issues with expanding this for general use and thus eliminating strptime/strftime.
As an aside, the txt/csv format currently used by garmin is incompatible with our implementation, which works with a historical garmin txt/csv format. I suspect our garmin_txt format is mostly useful as a playground for development and to get test coverage of certain features such as garmin_fs category support and init files.