From fa566c20429eaa85c62f5cd2d421359f7bb1ad76 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 18:31:56 +0900 Subject: [PATCH 1/6] refactor(rcs): declare "local i" closer --- completions/rcs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/completions/rcs b/completions/rcs index aab018e2473..108ffaac4f9 100644 --- a/completions/rcs +++ b/completions/rcs @@ -5,7 +5,7 @@ _comp_cmd_rcs() local cur prev words cword comp_args _comp_initialize -- "$@" || return - local file dir i + local file dir file=${cur##*/} dir=${cur%/*} @@ -23,6 +23,7 @@ _comp_cmd_rcs() _comp_compgen -aR -- -G "$dir/$file*,v" + local i for i in ${!COMPREPLY[*]}; do COMPREPLY[i]=${COMPREPLY[i]%,v} done From 0fc8fff69b4f0b5db44a56c94b84db450c79a4b2 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 18:32:27 +0900 Subject: [PATCH 2/6] fix(rcs): avoid unexpected pathname expansion 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. --- completions/rcs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/completions/rcs b/completions/rcs index 108ffaac4f9..bbf9c6b763d 100644 --- a/completions/rcs +++ b/completions/rcs @@ -21,7 +21,9 @@ _comp_cmd_rcs() COMPREPLY[i]=$dir$file done - _comp_compgen -aR -- -G "$dir/$file*,v" + local files + _comp_expand_glob files '"$dir/$file"*,v' && + _comp_compgen -aR -- -W '"${files[@]}"' local i for i in ${!COMPREPLY[*]}; do From e623bdb8810a25065f542c6f7b517279d11221b7 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 18:41:21 +0900 Subject: [PATCH 3/6] fix(mdtool): fix usage of `compgen -G` 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 73225ea4. In this patch, we remove `-f` and merge multiple `-G`. --- completions/mdtool | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/completions/mdtool b/completions/mdtool index cd179e5f436..aee955e44ac 100644 --- a/completions/mdtool +++ b/completions/mdtool @@ -21,10 +21,10 @@ _comp_cmd_mdtool() #if [[ "$prev" == *: ]]; then # case $prev in # @(--p:|--project:)) - # _comp_compgen -- -f -G "*.mdp" + # _comp_compgen -- -G "*.mdp" # ;; # @(--f:|--buildfile:)) - # _comp_compgen -- -f -G "*.mdp" -G "*.mds" + # _comp_compgen -- -G "*.md[ps]" # ;; # esac #fi From db1d239d1823e80bb88c21e00a4d22da71afccde Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 18:48:48 +0900 Subject: [PATCH 4/6] fix(mdtool): use `-f -X '!pat'` instead of `-G pat` https://github.com/scop/bash-completion/pull/1297#discussion_r2030093233 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. --- completions/mdtool | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/completions/mdtool b/completions/mdtool index aee955e44ac..22a409e5985 100644 --- a/completions/mdtool +++ b/completions/mdtool @@ -21,10 +21,10 @@ _comp_cmd_mdtool() #if [[ "$prev" == *: ]]; then # case $prev in # @(--p:|--project:)) - # _comp_compgen -- -G "*.mdp" + # _comp_compgen -- -f -X '!*.mdp' # ;; # @(--f:|--buildfile:)) - # _comp_compgen -- -G "*.md[ps]" + # _comp_compgen -- -f -X '!*.md[ps]' # ;; # esac #fi @@ -32,7 +32,7 @@ _comp_cmd_mdtool() ;; "generate-makefiles") compopt -o filenames - _comp_compgen -- -o filenames -G"*.mds" + _comp_compgen -- -o filenames -f -X '!*.mds' if [[ $prev == *mds ]]; then _comp_compgen -- -W '--simple-makefiles --s --d:' fi From 2c58e01c06fb7c571e8b5dd2087257379aa8d04e Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 18:48:48 +0900 Subject: [PATCH 5/6] docs(CONTRIBUTING): discourage use of `compgen -G pat` https://github.com/scop/bash-completion/pull/1297#discussion_r2030093233 --- CONTRIBUTING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d19b5cd7492..b9368f7492f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -132,6 +132,14 @@ Also, please bear the following coding guidelines in mind: expansions will be unexpectedly performed, which becomes a vulnerability. In the latter case, checks by shellcheck and shfmt will not be performed inside `'...'`. Also, `_comp_compgen_split` is `IFS`-safe. + + Avoid using `_comp_compgen -- -G "pattern"` to generate completions. The + result is not filtered by the current word `cur` due to the Bash design of + `compgen`. Also, this cannot be used to generate filenames with a specified + extension because the `-G` specification only generates the matching + filepaths in the current directory. It does not look into subdirectories + even when `$cur` implies completion in a subdirectory. One can instead use + `_comp_compgen -- -f -X '!pattern'`. - When completing available options, offer only the most descriptive ones as completion results if there are multiple options that do the From 272bbedb9cf3240dfb702f1cda052a83499da872 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sun, 6 Apr 2025 19:26:10 +0900 Subject: [PATCH 6/6] style(feh,upgradepkg,valgrind): remove unnecessary @(...) https://github.com/scop/bash-completion/pull/1297#discussion_r1899507348 --- completions/feh | 4 ++-- completions/upgradepkg | 2 +- completions/valgrind | 2 +- test/runLint | 3 +++ 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/completions/feh b/completions/feh index d9550519070..57a23c4bf83 100644 --- a/completions/feh +++ b/completions/feh @@ -29,13 +29,13 @@ _comp_cmd_feh() fi local font_path # font_path="$(imlib2-config --prefix 2>/dev/null)/share/imlib2/data/fonts" - # _comp_compgen -C "$font_path" -- -f -X "!*.@([tT][tT][fF])" -S / + # _comp_compgen -C "$font_path" -- -f -X "!*.[tT][tT][fF]" -S / for ((i = ${#words[@]} - 2; i > 0; i--)); do if [[ ${words[i]} == -@(C|-fontpath) ]]; then font_path="${words[i + 1]}" if [[ -d $font_path ]]; then _comp_compgen -aC "$font_path" -- \ - -f -X "!*.@([tT][tT][fF])" -S / + -f -X "!*.[tT][tT][fF]" -S / fi fi done diff --git a/completions/upgradepkg b/completions/upgradepkg index f359e9add19..0012f6282c3 100644 --- a/completions/upgradepkg +++ b/completions/upgradepkg @@ -15,7 +15,7 @@ _comp_cmd_upgradepkg() cur="${cur#*%}" local nofiles="" compopt -o filenames - _comp_compgen -- -P "$prev%" -f -X "!*.@(t[bgxl]z)" || nofiles=set + _comp_compgen -- -P "$prev%" -f -X "!*.t[bgxl]z" || nofiles=set _comp_compgen -a -- -P "$prev%" -S '/' -d [[ $nofiles ]] && compopt -o nospace return diff --git a/completions/valgrind b/completions/valgrind index 7043aa32fdb..12782922572 100644 --- a/completions/valgrind +++ b/completions/valgrind @@ -7,7 +7,7 @@ _comp_cmd_valgrind() local i for ((i = 1; i <= cword; i++)); do - if [[ ${words[i]} != @([-=])* ]]; then + if [[ ${words[i]} != [-=]* ]]; then _comp_command_offset $i return fi diff --git a/test/runLint b/test/runLint index 550995de147..d011736b18f 100755 --- a/test/runLint +++ b/test/runLint @@ -52,6 +52,9 @@ gitgrep '(?