New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be slightly less aggro about parser ambiguity wrt optional-value flags #516

Closed
bitprophet opened this Issue Apr 17, 2018 · 2 comments

Comments

Projects
None yet
1 participant
@bitprophet
Member

bitprophet commented Apr 17, 2018

Spinoff from #33, specifically #33 (comment)

Marking as bug but it's one of those things I'd only want to release in a minor release...esp as it's backwards incompatible. (Soon, so soon, this would be grounds for a major release! but we're still pre 1.0...)

tl;dr try reining in Parser.check_ambiguity() so it doesn't explode when the value given is flag-like, but it would be a valid flag for the current context: it seems more likely that the user was intending to give that 2nd flag (and implicitly NOT giving a value to the optional-value flag being parsed), than that they were intending to give such a flag as a value to this one.

We might also want to remove the check around another possible context/task name, but that one seems more truly ambiguous to me so prob best to leave it.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Apr 17, 2018

Also, while I am writing up the docs/changelogs for this, I realize - if we are happy resolving that particular ambiguity, it means the other half of it - flag-like values which are not otherwise valid for current context - can also be resolved (and preserved as the optional-value flag's string value.)

@bitprophet

This comment has been minimized.

Member

bitprophet commented Apr 17, 2018

But also, I now find myself wondering why all of this did not occur to me at the time. Is there something in the parser's design that made the current behavior way more compelling than the "are you valid or not?" test?

bitprophet added a commit that referenced this issue Apr 17, 2018

bitprophet added a commit that referenced this issue Apr 19, 2018

Realized there was a gap in test+impl coverage re #516
The --x=y form of value-bearing flags makes it look like
a non-valid flag. Gah. Stupid bug.

bitprophet added a commit that referenced this issue Apr 19, 2018

Missed a glaring spot re: #516
Specifically, valid flags that come after optional-value
flags, WHICH ARE IN --x=y FORM, did not pass muster due
to how parse tokenizing worked.

This should fix it and all the other tests still pass
so holy shit I am a genius or something???
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment