Skip to content

fix(mdtool,rcs,etc): fix uses of -G 'pat' and -f -X '!pat' #1358

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

Merged
merged 6 commits into from
Apr 9, 2025

Conversation

akinomyoga
Copy link
Collaborator

This PR addresses issues discussed in #1297. This PR also includes other changes. See the commit messages.

@akinomyoga
Copy link
Collaborator Author

There is still an issue with -f -X '!...' (which haven't been discussed in #1297). They do not generate directory names even though the user might want to specify a file inside a subdirectory. This is actually related to the problem discussed in #1348 (comment). I'm thinking of addressing it in another PR.

One of the reasons that some of the completion setting use -f X'!...' instead of calling filedir seems that those completions want to prefix or suffix additional string using -S / or -P "$prefix", which are unsupported by _comp_compgen_filedir.

  • One solution is to extend filedir to support these options -S and -P.
  • Another solution that I'm currently thinking is to extend _comp_compgen to accept -S and -P. Actually, I have an old local branch that implements these options (with non-trivial but necessary adjustments of cur). I'll try to resurrect and rebase the branch.

@akinomyoga akinomyoga changed the title fix(mdtool): fix uses of -G 'pat' and -f -X '!pat' fix(mdtool,rcs,etc): fix uses of -G 'pat' and -f -X '!pat' Apr 6, 2025
@akinomyoga akinomyoga requested a review from Copilot April 6, 2025 14:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (6)
  • completions/feh: Language not supported
  • completions/mdtool: Language not supported
  • completions/rcs: Language not supported
  • completions/upgradepkg: Language not supported
  • completions/valgrind: Language not supported
  • test/runLint: Language not supported

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.

Pre-approved with a couple of cosmetic notes, ok to merge whether addressed or not.

In the current implementation, when the current word contains any glob
characters, the glob pattern can match unexpected files.  We instead
use `_comp_expand_glob` with double-quoting the strings originating
from the current word.
The original code (which is commented out and marked as TODO) tried to
perform the filename generation by specifying both `-f` and `-G`, and
also specifying `-G` multiple times.  This has the following issues:

* The completion specification of `-f` and `-G` are independent, so
  this code is going to generate all filenames with `-f`. Then, it
  additionally generates the filenames matching the specified pattern.
* The `-G pat` can only be specified once, or the latter `-G pat`
  specification will overwrite the previous specification.  We should
  combine multiple patterns into one.

Note: This code stub was originally introduced in commit 73225ea.

In this patch, we remove `-f` and merge multiple `-G`.
scop#1297 (comment)

The compgen option `-X '!pat'` has a problem that it only generates
the filenames in the current directory and that it does not perform
the filtering by the "cur" specified to the argument of `compgen`:

    $ compgen -G '*.md' -- doc/
    CHANGELOG.md
    CONTRIBUTING.md
    README.md
    style.md

In this patch, we use `-f -X '!pat'`, which does not have those
limitations.
@akinomyoga akinomyoga force-pushed the compgen-G branch 2 times, most recently from e98f1a7 to 72f745f Compare April 9, 2025 10:51
@akinomyoga akinomyoga merged commit 461ae45 into scop:main Apr 9, 2025
7 checks passed
@akinomyoga akinomyoga deleted the compgen-G branch April 9, 2025 11:47
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