garmin, ozi, xcsv mkshort options whitespace_ok, must_unique broken since 2006 #982
Comments
|
I was able to rebuild 1.6.0 (2019) and verify the non-functionality of those two options with ozi and xcsv. |
|
I was also able to build 1.5.4 (2017), with a few tweaks, and verify the same non-functionality. |
|
Agreed. After even five years,. It's not a bug; it's a feeeeeature.
…On Thu, Jan 12, 2023, 4:02 PM tsteven4 ***@***.***> wrote:
I was also able to build 1.5.4 (2017), with a few tweaks, and verify the
same non-functionality.
—
Reply to this email directly, view it on GitHub
<#982 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC3VAD7OZMUNSK4LXB7OBG3WSB5N7ANCNFSM6AAAAAATZW32IE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
@robertlipe The snupper* options did work, so the lack of a need for them is unproven. I contend the others have been ignored worked for 16 years. |
|
I agree. These are sensible defaults. We probably should have never exposed
knobs for these.
Changing xcsv is always scary as it trickles to so many (often obscure and
rarely testable for us) formats, but since anything using it has used these
settings for so many years, I agree.
Launch codes verified. Execute at your call.
…On Thu, Jan 12, 2023, 5:41 PM tsteven4 ***@***.***> wrote:
@robertlipe <https://github.com/robertlipe>
Specifically I propose to delete
ozi snwhite, snunique and snupper*
garmin snwhite
xcsv snwhite, snunique and snupper*
options.
The snupper* options did work, so the lack of a need for them is unproven.
I contend the others have been ignored worked for 16 years.
—
Reply to this email directly, view it on GitHub
<#982 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD34W2MC2TYXNMFUUB7TWSCJBDANCNFSM6AAAAAATZW32IE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I'm ok with the idea, but #1239 has a bunch of merge conflicts. |
|
I think my work tree is just corrupt. It's time to just rm -fr it and check
out a new working copy. I'm getting merge conflicts in code that's not been
touched in years that doesn't show up on a diff or even after I first
commit it.
Though I admit to having a strained relationship with Git, it does not fill
me with love and confidence. :-/
I'll work through that separately, though it may be a bit with the holidays.
…On Mon, Nov 20, 2023 at 1:04 PM tsteven4 ***@***.***> wrote:
I'm ok with the idea, but #1239
<#1239> has a bunch of merge
conflicts.
—
Reply to this email directly, view it on GitHub
<#982 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD3Z3ZAIQHRTMTMUGKNLYFOSUJAVCNFSM6AAAAAATZW32IGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZGY2DCMBRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
#1239 looks like a bad merge. Sorry, I have touched a ton of stuff in the last week converting the last formats to Format class, including garmin. |
|
That was a fresh pull from head and a branch created last night. I edited
that merge conflict (like 4 - 5 files) on the web and got 418 changed
files. Clearly my git repo is screwed up somewhere.
…On Mon, Nov 20, 2023 at 5:45 PM tsteven4 ***@***.***> wrote:
#1239 <#1239> looks like a bad
merge. Sorry, I have touched a ton of stuff in the last week converting the
last formats to Format class, including garmin.
—
Reply to this email directly, view it on GitHub
<#982 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD3YKX5ZHVKQUZZ47MZTYFPTTDAVCNFSM6AAAAAATZW32IGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZHE4DINZQGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I think 75a4b039b broke these options by adding the logic in vecs noted by
/* for bool options without default: don't set argval if "FALSE" */at that time, as today, mkshort_new_handle set both options to 1 (true).
garmin.c, ozi.c and xcsv.c wouldn't call setshort_whitespace_ok or setshort_mustuniq if the option value was null. They all used bool options without a default. Therefore, it was impossible to use those options to set whitespace_ok to false or must_unique to false.
If so this went undiscovered for 16 years. This suggest to me these options can be removed from these formats instead of fixed.
The option must_upper defaulted to 0 in mkshort_new_handle. Therefore the format options to set this could have worked. I have verified that they don't work today with ozi and xcsv. I haven't verified that the above commit broke them, or that they stayed broken for every release since then.
I would propose removing the format options for whitespaceok and must_unique from garmin, ozi and xcsv. We could further consider removing the option for must_upper, although that worked and could be in use.
The text was updated successfully, but these errors were encountered: