From a16b7c58f9bc377b47af94c4d0c8218e7e4e9777 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 13:08:25 +0900 Subject: [PATCH 1/8] fix(scp): fix a bug that `-F[TAB]` did not complete at all This was because the filtering by a modified `cur` is performed after prefixing "-F". Since we generate the filenames starting with "$cur", we do not need to further filter the results with `cur`. We disable the filtering by specifying the `-R` flag to `_comp_compgen`. --- completions/ssh | 4 ++-- test/t/test_scp.py | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/completions/ssh b/completions/ssh index b8e84b38f88..db30ba340f7 100644 --- a/completions/ssh +++ b/completions/ssh @@ -547,13 +547,13 @@ _comp_xfunc_scp_compgen_local_files() local files _comp_expand_glob files '"$cur"*' || return 0 if [[ $_dirsonly ]]; then - _comp_compgen -U files split -l -- "$( + _comp_compgen -RU files split -l -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ -e '/[^\/]$/d' -e "s/^/${1-}/" )" else - _comp_compgen -U files split -l -- "$( + _comp_compgen -RU files split -l -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ -e 's/[*@|=]$//g' -e 's/[^\/]$/& /g' -e "s/^/${1-}/" diff --git a/test/t/test_scp.py b/test/t/test_scp.py index b91e5476827..31a588d80ff 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -55,6 +55,14 @@ def test_capital_f_without_space(self, completion): "option requires an argument -- F" in x for x in completion ) + @pytest.mark.complete("scp -Fconf", cwd="scp") + def test_capital_f_without_space_2(self, completion): + assert completion == "ig" + + @pytest.mark.complete("scp -Fbi", cwd="scp") + def test_capital_f_without_space_3(self, completion): + assert completion == "n/" + @pytest.fixture(scope="class") def live_pwd(self, bash): try: From 837a009d332991781982172b50ece5c2dac2f6f3 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 16:12:00 +0900 Subject: [PATCH 2/8] fix(rsync,ssh,sshfs): do not generate regular files *'\' as dirs In sed, /[^\/]/ does not mean "a character that is not a slash". It means "a character that is not a backslash or a slash". In the bracket expressions [...], slashes do not need to be escaped by a backslash, and also, a backslash is treated literally. We should instead use /[^/]/. --- completions/ssh | 8 ++++---- test/fixtures/sshfs/local_path-dir/dummy.txt | 0 "test/fixtures/sshfs/local_path-file\\" | 0 test/t/test_sshfs.py | 4 ++++ 4 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/sshfs/local_path-dir/dummy.txt create mode 100644 "test/fixtures/sshfs/local_path-file\\" diff --git a/completions/ssh b/completions/ssh index db30ba340f7..7a8a98e0ce0 100644 --- a/completions/ssh +++ b/completions/ssh @@ -512,7 +512,7 @@ _comp_xfunc_scp_compgen_remote_files() # shellcheck disable=SC2090 _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | - command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e '/[^\/]$/d') + command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e '/[^/]$/d') else # escape problematic characters; remove executables, aliases, pipes # and sockets; add space at end of file names @@ -520,7 +520,7 @@ _comp_xfunc_scp_compgen_remote_files() _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e 's/[*@|=]$//g' \ - -e 's/[^\/]$/& /g') + -e 's/[^/]$/& /g') fi _comp_compgen -R split -l -- "$_files" } @@ -550,13 +550,13 @@ _comp_xfunc_scp_compgen_local_files() _comp_compgen -RU files split -l -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e '/[^\/]$/d' -e "s/^/${1-}/" + -e '/[^/]$/d' -e "s/^/${1-}/" )" else _comp_compgen -RU files split -l -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e 's/[*@|=]$//g' -e 's/[^\/]$/& /g' -e "s/^/${1-}/" + -e 's/[*@|=]$//g' -e 's/[^/]$/& /g' -e "s/^/${1-}/" )" fi } diff --git a/test/fixtures/sshfs/local_path-dir/dummy.txt b/test/fixtures/sshfs/local_path-dir/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git "a/test/fixtures/sshfs/local_path-file\\" "b/test/fixtures/sshfs/local_path-file\\" new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/t/test_sshfs.py b/test/t/test_sshfs.py index cb4189bf5e1..71977adf367 100644 --- a/test/t/test_sshfs.py +++ b/test/t/test_sshfs.py @@ -6,3 +6,7 @@ class TestSshfs: @pytest.mark.complete("sshfs ./") def test_1(self, completion): assert completion + + @pytest.mark.complete("sshfs local_path", cwd="sshfs") + def test_local_path_suffix_1(self, completion): + assert completion == "-dir/" From d53b83093967c8072c49ee35264b2792dc41f651 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 16:55:25 +0900 Subject: [PATCH 3/8] fix(rsync,scp): remove file-type marks from "ls -F" properly The type-classifier characters for named pipes (|) and sockets (=) were not properly removed. These are escaped by a backslash before removing, so the backslashes are left. In this patch, we remove the type-classifier characters before performing the backslash escaping. --- completions/ssh | 8 +++++--- test/t/test_scp.py | 26 +++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/completions/ssh b/completions/ssh index 7a8a98e0ce0..279c44b040a 100644 --- a/completions/ssh +++ b/completions/ssh @@ -519,7 +519,8 @@ _comp_xfunc_scp_compgen_remote_files() # shellcheck disable=SC2090 _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | - command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e 's/[*@|=]$//g' \ + command sed -e 's/[*@|=]$//g' \ + -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' \ -e 's/[^/]$/& /g') fi _comp_compgen -R split -l -- "$_files" @@ -555,8 +556,9 @@ _comp_xfunc_scp_compgen_local_files() else _comp_compgen -RU files split -l -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | - command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e 's/[*@|=]$//g' -e 's/[^/]$/& /g' -e "s/^/${1-}/" + command sed -e 's/[*@|=]$//g' \ + -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ + -e 's/[^/]$/& /g' -e "s/^/${1-}/" )" fi } diff --git a/test/t/test_scp.py b/test/t/test_scp.py index 31a588d80ff..e630bbdc9f9 100644 --- a/test/t/test_scp.py +++ b/test/t/test_scp.py @@ -2,7 +2,12 @@ import pytest -from conftest import assert_bash_exec, assert_complete, bash_env_saved +from conftest import ( + assert_bash_exec, + assert_complete, + bash_env_saved, + prepare_fixture_dir, +) LIVE_HOST = "bash_completion" @@ -149,3 +154,22 @@ def test_xfunc_remote_files(self, bash): "shared/default/foo ", "shared/default/foo.d/", ] + + @pytest.fixture + def tmpdir_mkfifo(self, request, bash): + tmpdir, _, _ = prepare_fixture_dir(request, files=[], dirs=[]) + + try: + assert_bash_exec(bash, "mkfifo '%s/local_path_1-pipe'" % tmpdir) + except Exception: + pytest.skip( + "The present system does not allow creating a named pipe." + ) + + return tmpdir + + def test_local_path_mark_1(self, bash, tmpdir_mkfifo): + completion = assert_complete( + bash, "scp local_path_1-", cwd=tmpdir_mkfifo + ) + assert completion == "pipe" From 71f860b06f67694108ebd82aabd0e76ec5bcbf99 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 15:37:56 +0900 Subject: [PATCH 4/8] refactor(ssh): remove outdated disable=SC2089,SC2090 These shellcheck directives were added in commit c906aeb (2020-04-10), but the latest version of shellcheck-0.10.0 does not seem to warn about SC2089 and SC2090 on these lines for the current version of bash_completion. These warnings seem to be issued when unquoted $_comp_cmd_scp__path_esc is used, such as echo $_comp_cmd_scp__path_esc We have already resolved those unquoted variable expansions, so the warning should no longer happen. In this patch, we remove those outdated shellcheck directives. --- completions/ssh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/completions/ssh b/completions/ssh index 279c44b040a..8df5f3c96f0 100644 --- a/completions/ssh +++ b/completions/ssh @@ -459,7 +459,6 @@ _comp_cmd_sftp() shopt -u hostcomplete && complete -F _comp_cmd_sftp sftp # things we want to backslash escape in scp paths -# shellcheck disable=SC2089 _comp_cmd_scp__path_esc='[][(){}<>"'"'"',:;^&!$=?`\\|[:space:]]' # Complete remote files with ssh. Returns paths escaped with three backslashes @@ -493,7 +492,6 @@ _comp_xfunc_scp_compgen_remote_files() local _path=${cur#*:} # unescape (3 backslashes to 1 for chars we escaped) - # shellcheck disable=SC2090 _path=$(command sed -e 's/\\\\\\\('"$_comp_cmd_scp__path_esc"'\)/\\\1/g' <<<"$_path") # default to home dir of specified user on remote host @@ -509,14 +507,12 @@ _comp_xfunc_scp_compgen_remote_files() local _files if [[ $_dirs_only ]]; then # escape problematic characters; remove non-dirs - # shellcheck disable=SC2090 _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | command sed -e 's/'"$_comp_cmd_scp__path_esc"'/'"$_escape_replacement"'/g' -e '/[^/]$/d') else # escape problematic characters; remove executables, aliases, pipes # and sockets; add space at end of file names - # shellcheck disable=SC2090 _files=$(ssh -o 'Batchmode yes' "$_userhost" \ command ls -aF1dL "$_path*" 2>/dev/null | command sed -e 's/[*@|=]$//g' \ From c54a0f368491edcde55899992701be4a9936278d Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 13:31:43 +0900 Subject: [PATCH 5/8] refactor(scp): use "_comp_compgen_split -P" to add prefix --- completions/ssh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/completions/ssh b/completions/ssh index 8df5f3c96f0..236e16a3775 100644 --- a/completions/ssh +++ b/completions/ssh @@ -544,17 +544,17 @@ _comp_xfunc_scp_compgen_local_files() local files _comp_expand_glob files '"$cur"*' || return 0 if [[ $_dirsonly ]]; then - _comp_compgen -RU files split -l -- "$( + _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e '/[^/]$/d' -e "s/^/${1-}/" + -e '/[^/]$/d' )" else - _comp_compgen -RU files split -l -- "$( + _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e 's/[*@|=]$//g' \ -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ - -e 's/[^/]$/& /g' -e "s/^/${1-}/" + -e 's/[^/]$/& /g' )" fi } From 023f4a9aacd08330f0d036747955a3d8e5b1e38e Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Sat, 5 Apr 2025 22:21:13 +0900 Subject: [PATCH 6/8] fix(_comp_command_offset): work around nounset --- bash_completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bash_completion b/bash_completion index 89a9584be12..cf16b672924 100644 --- a/bash_completion +++ b/bash_completion @@ -2856,7 +2856,7 @@ _comp_command_offset() _comp_compgen_commands else _comp_dequote "${COMP_WORDS[0]}" || REPLY=${COMP_WORDS[0]} - local cmd=$REPLY compcmd=$REPLY + local cmd=${REPLY-} compcmd=${REPLY-} local cspec=$(complete -p -- "$cmd" 2>/dev/null) # If we have no completion for $cmd yet, see if we have for basename From ca2d563180cbbb86a6d852f0e3d61de593ea9e24 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Mon, 7 Apr 2025 08:15:50 +0900 Subject: [PATCH 7/8] refactor(scp): rename local var "_dirs{ => _}only" for consistency --- completions/ssh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/completions/ssh b/completions/ssh index 236e16a3775..55753333684 100644 --- a/completions/ssh +++ b/completions/ssh @@ -535,15 +535,15 @@ _scp_remote_files() # @since 2.12 _comp_xfunc_scp_compgen_local_files() { - local _dirsonly="" + local _dirs_only="" if [[ ${1-} == -d ]]; then - _dirsonly=set + _dirs_only=set shift fi local files _comp_expand_glob files '"$cur"*' || return 0 - if [[ $_dirsonly ]]; then + if [[ $_dirs_only ]]; then _comp_compgen -RU files split -l ${1:+-P "$1"} -- "$( command ls -aF1dL "${files[@]}" 2>/dev/null | command sed -e "s/$_comp_cmd_scp__path_esc/\\\\&/g" \ From ce498b979425cff92a052c9abfd12533849847b3 Mon Sep 17 00:00:00 2001 From: Koichi Murase Date: Mon, 7 Apr 2025 09:06:15 +0900 Subject: [PATCH 8/8] test(cancel,ls,man): remove redundant "return" pytest.skip cancels the execution of the current test, so "return" and subsequent codes arex unreachable. We do not need to explicitly call "return" in this context. https://github.com/scop/bash-completion/pull/1357#discussion_r2030308800 --- test/t/test_cancel.py | 2 +- test/t/test_ls.py | 2 +- test/t/test_man.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/t/test_cancel.py b/test/t/test_cancel.py index 4aeafd2c5fa..9384908c630 100644 --- a/test/t/test_cancel.py +++ b/test/t/test_cancel.py @@ -16,7 +16,7 @@ def added_job(self, request, bash): ) except AssertionError: pytest.skip("Could not add test print job") - return + if len(got) > 3: request.addfinalizer( lambda: assert_bash_exec(bash, "cancel %s" % got[3]) diff --git a/test/t/test_ls.py b/test/t/test_ls.py index f91ee6b2e2e..56f0ee71e90 100644 --- a/test/t/test_ls.py +++ b/test/t/test_ls.py @@ -33,7 +33,7 @@ def test_3(self, bash): part_full = find_unique_completion_pair(res) if not part_full: pytest.skip("No suitable test user found") - return + part, full = part_full completion = assert_complete(bash, "ls ~%s" % part) assert completion == full[len(part) :] diff --git a/test/t/test_man.py b/test/t/test_man.py index 081b8fcc1e7..bec41de9613 100644 --- a/test/t/test_man.py +++ b/test/t/test_man.py @@ -23,7 +23,7 @@ def colonpath(self, request, bash): pass else: pytest.skip("Cygwin doesn't like paths with colons") - return + tmpdir, _, _ = prepare_fixture_dir( request, files=["man/man3/Bash::Completion.3pm.gz"],