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(_umount,feh,sbopkg): check diretory name for _comp_compgen -C #1297

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

akinomyoga
Copy link
Collaborator

@akinomyoga akinomyoga commented Dec 30, 2024

Fixes #1260

This is an alternative fix to #1290. The fix suggested in #1290 was just to remove the error message, which I believe is not the right fix. This PR fixes it by checking the arguments at the caller side.

This includes an additional fix e9e665e for feh. In the current main branch, an error message is printed on the TAB completion with the following command line:

$ set -u
$ feh --font x[TAB] --fontpath
bash: words[i + 1]: unbound variable

In the current master, an error message is printed on the TAB
completion with the following command line (where the cursor position
is placed before the marker "[TAB]"):

$ set -u
$ feh --font x[TAB] --fontpath
bash: words[i + 1]: unbound variable
* fix(_umount): specify the correct path as a directory
* fix(feh): check the directory name before attempting path completion
* fix(sbopkg): check the QUEUEDIR value before generating *.sqf
-f -X "!*.@([tT][tT][fF])" -S /
if [[ -d $font_path ]]; then
_comp_compgen -aC "$font_path" -- \
-f -X "!*.@([tT][tT][fF])" -S /
Copy link
Collaborator Author

@akinomyoga akinomyoga Dec 30, 2024

Choose a reason for hiding this comment

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

Unrelated to the present topic, but I'm wondering about the reasoning for using -f -X "!globpat" instead of simply -G "globpat". In the codebase, we only have two uses of -G globpat:

./completions/mdtool:35:                _comp_compgen -- -o filenames -G"*.mds"
./completions/rcs:24:    _comp_compgen -aR -- -G "$dir/$file*,v"

while we have many for -f -X "!globpat":

./completions/explodepkg:3:complete -o plusdirs -f -X '!*.t[bglx]z' explodepkg
./completions/feh:32:            # _comp_compgen -C "$font_path" -- -f -X "!*.@([tT][tT][fF])" -S /
./completions/feh:38:                            -f -X "!*.@([tT][tT][fF])" -S /
./completions/gdb:41:        _comp_compgen -a -- -f -X '!?(*/)core?(.?*)' -o plusdirs
./completions/java:87:            _comp_compgen -av tmp -c "$i/$cur" -- -f -X '!*.class'
./completions/lzip:47:    _comp_compgen -- -f -X "*.lz" -o plusdirs
./completions/sbopkg:67:        _comp_compgen -aC "$QUEUEDIR" -- -f -X "!*.sqf"
./completions/_slackpkg:68:                _comp_compgen -C "$confdir/templates" -- -f -X \
                    "!?*.template" && COMPREPLY=("${COMPREPLY[@]%.template}")
./completions/upgradepkg:18:        _comp_compgen -- -P "$prev%" -f -X "!*.@(t[bgxl]z)" || nofiles=set

edit: One possibile explanation is that they derived from the pattern -f -X "$_xspec" used in _comp_compgen_filedir. In that case, we can probably use -G.

edit: I also checked the history of the support for -G by the compgen builtin, but the option -G seems to have existed from the beginning when compgen was introduced in Bash 2.04 (1999).

Copy link
Collaborator Author

@akinomyoga akinomyoga Dec 30, 2024

Choose a reason for hiding this comment

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

They were introduced in the following commits, where I don't find any discussions about using -f -X over -G.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also think the following @(...) are not needed (where ... does not contain |):

./completions/feh:32:            # _comp_compgen -C "$font_path" -- -f -X "!*.@([tT][tT][fF])" -S /
./completions/feh:38:                            -f -X "!*.@([tT][tT][fF])" -S /
./completions/upgradepkg:18:        _comp_compgen -- -P "$prev%" -f -X "!*.@(t[bgxl]z)" || nofiles=set
./completions/valgrind:10:        if [[ ${words[i]} != @([-=])* ]]; then

@akinomyoga akinomyoga changed the title Compgen cwd fix(_umount,feh,sbopkg): check diretory name for _comp_compgen -C Dec 30, 2024
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.

Error occurred when completing the "umount" command
1 participant