Skip to content
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

fix(completions/*): skip prev of the form -oOPTARG #862

Merged
merged 3 commits into from
Dec 22, 2022

Conversation

akinomyoga
Copy link
Collaborator

@akinomyoga akinomyoga commented Dec 19, 2022

I'm not sure if this is what was intended, but this is a possible fix to #852 (comment) (which is causing the test failures in #861 and #546). This properly handles the completion of the form -oOPTARG [TAB], yet this does not handle the completion of the form -oOPTAR[TAB].

I also thought about the possibility of modifying __reassemble_comp_words_by_ref so that -o and OPTARG in a single word -oOPTARG would be generated as separate words in words, but this can make the processing more complicated. For example, we need to prefix -o for the completion of OPTARG if -o and OPTARG originally come from a single word, etc.

Edit:

  • Commit 1fcbe44 is an additional change to the pytest completion.

Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's just a huge amount of work you've put into this, thanks! And yes, this is what I meant in #852.

I guess the only thing I'd change here would be to use something else than otherflags for the variable name, because it's not "other" as the set of flag names in it will contain also the one being processed (e.g. for -${otherflags}d there should be a d also in ${otherflags}), and it doesn't capture the reason for its existence in the name.

noargopts springs to mind. This would read nicely like -${noargopts}d -- roughly "dash followed by options that do not take an arg, followed by d".

@akinomyoga
Copy link
Collaborator Author

akinomyoga commented Dec 20, 2022

Thank you! I have switched to noargopts.

I actually initially thought about flags (to me, which imply the options that do not take option arguments), but the variable name was already used. Another possibility might be flagopts, but it is not explicit that they do not receive option arguments.

@scop scop merged commit a392580 into scop:master Dec 22, 2022
@scop
Copy link
Owner

scop commented Dec 22, 2022

Nice! Merged.

Very subtle, but the reason I have a slight preference on "opts" instead of "flags" here is that the glob we use for this contains also the prevention to match long options (and I don't know if "long flags" is a thing, at least it sounds odd to me).

@akinomyoga akinomyoga deleted the reportbug-otherflags branch December 22, 2022 07:55
@akinomyoga
Copy link
Collaborator Author

Thanks!

the glob we use for this contains also the prevention to match long options

I see. That makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants