Merge pull request #924 from scop/feat/load-completion-improvements

feat: load completion improvements

bash_completion

@@ -1713,6 +1713,25 @@ _fstypes()
     [[ $fss ]] && _comp_compgen -a COMPREPLY -W "$fss" -- "$cur"
 }
 
+# Get absolute path to a file, with rudimentary canonicalization.
+# No symlink resolution or existence checks are done;
+# see `_comp_realcommand` for those.
+# @param $1 The file
+# @var[out] ret The path
+_comp_abspath()
+{
+    ret=$1
+    case $ret in
+        /*) ;;
+        ../*) ret=$PWD/${ret:3} ;;
+        *) ret=$PWD/$ret ;;
+    esac
+    while [[ $ret == */./* ]]; do
+        ret=${ret//\/.\//\/}
+    done
+    ret=${ret//+(\/)/\/}
+}
+
 # Get real command.
 # Command is the filename of command in PATH with possible symlinks resolved
 # (if resolve tooling available), empty string if command not found.
@@ -1731,16 +1750,7 @@ _comp_realcommand()
     elif type -p readlink >/dev/null; then
         ret=$(readlink -f "$file")
     else
-        ret=$file
-        if [[ $ret == */* ]]; then
-            if [[ $ret == ./* ]]; then
-                ret=$PWD/${file:2}
-            elif [[ $ret == ../* ]]; then
-                ret=$PWD/${file:3}
-            elif [[ $ret != /* ]]; then
-                ret=$PWD/$file
-            fi
-        fi
+        _comp_abspath "$file"
     fi
 }
 
@@ -2526,9 +2536,24 @@ complete -F _minimal ''
 
 __load_completion()
 {
-    local cmd="${1##*/}" dir compfile
+    local cmd=$1 cmdname=${1##*/} dir compfile
     local -a paths
-    [[ $cmd ]] || return 1
+    [[ $cmdname ]] || return 1
+
+    local backslash=
+    if [[ $cmd == \\* ]]; then
+        cmd=${cmd:1}
+        # If we already have a completion for the "real" command, use it
+        $(complete -p "$cmd" 2>/dev/null || echo false) "\\$cmd" && return 0
+        backslash=\\
+    fi
+
+    # Resolve absolute path to $cmd
+    local ret pathcmd origcmd=$cmd
+    if pathcmd=$(type -P "$cmd"); then
+        _comp_abspath "$pathcmd"
+        cmd=$ret
+    fi
 
     local -a dirs=()
 
@@ -2553,68 +2578,75 @@ __load_completion()
         dirs+=(./completions)
     fi
 
-    # 3) From bin directories extracted from $(realpath "$cmd") and PATH
+    # 3) From bin directories extracted from the specified path to the command,
+    # the real path to the command, and $PATH
+    paths=()
+    [[ $cmd == /* ]] && paths+=("${cmd%/*}")
     local ret
-    _comp_realcommand "$1" && paths=("${ret%/*}") || paths=()
+    _comp_realcommand "$cmd" && paths+=("${ret%/*}")
     _comp_split -aF : paths "$PATH"
     for dir in "${paths[@]%/}"; do
-        if [[ -d $dir && $dir == ?*/@(bin|sbin) ]]; then
+        [[ $dir == ?*/@(bin|sbin) ]] &&
             dirs+=("${dir%/*}/share/bash-completion/completions")
-        fi
     done
 
     # 4) From XDG_DATA_DIRS or system dirs (e.g. /usr/share, /usr/local/share):
     # Completions in the system data dirs.
     _comp_split -F : paths "${XDG_DATA_DIRS:-/usr/local/share:/usr/share}" &&
         dirs+=("${paths[@]/%//bash-completion/completions}")
 
-    local backslash=
-    if [[ $cmd == \\* ]]; then
-        cmd=${cmd:1}
-        # If we already have a completion for the "real" command, use it
-        $(complete -p "$cmd" 2>/dev/null || echo false) "\\$cmd" && return 0
-        backslash=\\
-    fi
-
-    # For loading 3rd party completions wrapped in shopt reset
+    # Set up default $IFS in case loaded completions depend on it,
+    # as well as for $compspec invocation below.
     local IFS=$' \t\n'
 
-    for dir in "${dirs[@]}"; do
-        [[ -d $dir ]] || continue
-        for compfile in "$cmd" "$cmd.bash"; do
-            compfile="$dir/$compfile"
-            # Avoid trying to source dirs as long as we support bash < 4.3
-            # to avoid an fd leak; https://bugzilla.redhat.com/903540
-            if [[ -d $compfile ]]; then
-                # Do not warn with . or .. (especially the former is common)
-                [[ $compfile == */.?(.) ]] ||
-                    echo "bash_completion: $compfile: is a directory" >&2
-            elif [[ -e $compfile ]] && . "$compfile"; then
-                [[ $backslash ]] && $(complete -p "$cmd") "\\$cmd"
-                return 0
+    # Look up and source
+    shift
+    local i prefix compspec
+    for prefix in "" _; do # Regular from all dirs first, then fallbacks
+        for i in ${!dirs[*]}; do
+            dir=${dirs[i]}
+            if [[ ! -d $dir ]]; then
+                unset -v 'dirs[i]'
+                continue
             fi
+            for compfile in "$prefix$cmdname" "$prefix$cmdname.bash"; do
+                compfile="$dir/$compfile"
+                # Avoid trying to source dirs as long as we support bash < 4.3
+                # to avoid an fd leak; https://bugzilla.redhat.com/903540
+                if [[ -d $compfile ]]; then
+                    # Do not warn with . or .. (especially the former is common)
+                    [[ $compfile == */.?(.) ]] ||
+                        echo "bash_completion: $compfile: is a directory" >&2
+                elif [[ -e $compfile ]] && . "$compfile" "$cmd" "$@"; then
+                    # At least $cmd is expected to have a completion set when
+                    # we return successfully; see if it already does
+                    if compspec=$(complete -p "$cmd" 2>/dev/null); then
+                        local -a extspecs=()
+                        # $cmd is the case in which we do backslash processing
+                        [[ $backslash ]] && extspecs+=("$backslash$cmd")
+                        # If invoked without path, that one should be set, too
+                        # ...but let's not overwrite an existing one, if any
+                        [[ $origcmd != */* ]] &&
+                            ! complete -p "$origcmd" &>/dev/null &&
+                            extspecs+=("$origcmd")
+                        ((${#extspecs[*]} != 0)) && $compspec "${extspecs[@]}"
+                        return 0
+                    fi
+                    # If not, see if we got one for $cmdname
+                    if [[ $cmdname != "$cmd" ]] && compspec=$(complete -p "$cmdname" 2>/dev/null); then
+                        # Use that for $cmd too, if we have a full path to it
+                        [[ $cmd == /* ]] && $compspec "$cmd"
+                        return 0
+                    fi
+                    # Nothing expected was set, continue lookup
+                fi
+            done
         done
     done
 
-    # Search fallback completions named "_$cmd"
-    for dir in "${dirs[@]}"; do
-        [[ -d $dir ]] || continue
-        compfile="$dir/_$cmd"
-        # Avoid trying to source dirs as long as we support bash < 4.3
-        # to avoid an fd leak; https://bugzilla.redhat.com/903540
-        if [[ -d $compfile ]]; then
-            # Do not warn with . or .. (especially the former is common)
-            [[ $compfile == */.?(.) ]] ||
-                echo "bash_completion: $compfile: is a directory" >&2
-        elif [[ -e $compfile ]] && . "$compfile" "$cmd"; then
-            [[ $backslash ]] && $(complete -p "$cmd") "\\$cmd"
-            return 0
-        fi
-    done
-
     # Look up simple "xspec" completions
-    [[ -v _xspecs[$cmd] ]] &&
-        complete -F _filedir_xspec "$cmd" "$backslash$cmd" && return 0
+    [[ -v _xspecs[$cmdname] ]] &&
+        complete -F _filedir_xspec "$cmdname" "$backslash$cmdname" && return 0
 
     return 1
 }

completions/_cargo

@@ -6,8 +6,4 @@
 local rustup="${1%cargo}rustup" # use rustup from same dir
 eval -- "$("$rustup" completions bash cargo 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

completions/_gh

@@ -5,8 +5,4 @@
 
 eval -- "$("$1" completion --shell bash 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

completions/_golangci-lint

@@ -6,8 +6,4 @@
 
 eval -- "$("$1" completion bash 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

completions/_nox

@@ -10,6 +10,4 @@ eval -- "$(
         register-python-argcomplete3 --shell bash "$1" 2>/dev/null
 )"
 
-complete -p "$1" &>/dev/null
-
 # ex: filetype=sh

completions/_ruff

@@ -5,8 +5,4 @@
 
 eval -- "$("$1" generate-shell-completion bash 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

completions/_rustup

@@ -4,8 +4,4 @@
 
 eval -- "$("$1" completions bash rustup 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

completions/_vault

@@ -3,6 +3,6 @@
 #
 # This serves as a fallback in case the completion is not installed otherwise.
 
-type "$1" &>/dev/null && complete -C "\"$1\" 2>/dev/null" "$1" "${1##*/}"
+type "$1" &>/dev/null && complete -C "\"$1\" 2>/dev/null" "$1"
 
 # ex: filetype=sh

completions/_yq

@@ -5,8 +5,4 @@
 
 eval -- "$("$1" shell-completion bash 2>/dev/null)"
 
-{
-    complete -p "$1" || complete -p "${1##*/}"
-} &>/dev/null
-
 # ex: filetype=sh

test/fixtures/__load_completion/prefix1/share/bash-completion/completions/cmd1

@@ -1 +1,2 @@
 echo 'cmd1: sourced from prefix1'
+complete -C true "$1"

test/fixtures/__load_completion/prefix1/share/bash-completion/completions/cmd2

@@ -1 +1,2 @@
 echo 'cmd2: sourced from prefix1'
+complete -C true "$1"

test/fixtures/__load_completion/userdir1/completions/cmd1

@@ -1 +1,2 @@
 echo 'cmd1: sourced from userdir1'
+complete -C true "$1"

test/fixtures/__load_completion/userdir2/completions/cmd2

@@ -1 +1,2 @@
 echo 'cmd2: sourced from userdir2'
+complete -C true "$1"

test/t/unit/Makefile.am

@@ -1,4 +1,5 @@
 EXTRA_DIST = \
+	test_unit_abspath.py \
 	test_unit_command_offset.py \
 	test_unit_compgen.py \
 	test_unit_count_args.py \

test/t/unit/test_unit_abspath.py

@@ -0,0 +1,67 @@
+import pytest
+
+from conftest import assert_bash_exec
+
+
+@pytest.mark.bashcomp(
+    cmd=None, cwd="shared", ignore_env=r"^\+declare -f __tester$"
+)
+class TestUnitAbsPath:
+    @pytest.fixture
+    def functions(self, bash):
+        assert_bash_exec(
+            bash,
+            (
+                "__tester() { "
+                "local ret; "
+                '_comp_abspath "$1"; '
+                'printf %s "$ret"; '
+                "}"
+            ),
+        )
+
+    def test_non_pollution(self, bash):
+        """Test environment non-pollution, detected at teardown."""
+        assert_bash_exec(
+            bash,
+            "foo() { local ret=; _comp_abspath bar; }; foo; unset -f foo",
+            want_output=None,
+        )
+
+    def test_absolute(self, bash, functions):
+        output = assert_bash_exec(
+            bash,
+            "__tester /foo/bar",
+            want_output=True,
+            want_newline=False,
+        )
+        assert output.strip() == "/foo/bar"
+
+    def test_relative(self, bash, functions):
+        output = assert_bash_exec(
+            bash,
+            "__tester foo/bar",
+            want_output=True,
+            want_newline=False,
+        )
+        assert output.strip().endswith("/shared/foo/bar")
+
+    def test_cwd(self, bash, functions):
+        output = assert_bash_exec(
+            bash,
+            "__tester ./foo/./bar",
+            want_output=True,
+            want_newline=False,
+        )
+        assert output.strip().endswith("/shared/foo/bar")
+
+    def test_parent(self, bash, functions):
+        output = assert_bash_exec(
+            bash,
+            "__tester ../shared/foo/bar",
+            want_output=True,
+            want_newline=False,
+        )
+        assert output.strip().endswith(
+            "/shared/foo/bar"
+        ) and not output.strip().endswith("../shared/foo/bar")

test/t/unit/test_unit_load_completion.py

@@ -37,12 +37,26 @@ def test_PATH_1(self, bash):
                 bash, "__load_completion cmd2", want_output=True
             )
             assert output.strip() == "cmd2: sourced from prefix1"
+            output = assert_bash_exec(
+                bash, "complete -p cmd2", want_output=True
+            )
+            assert " cmd2" in output
+            output = assert_bash_exec(
+                bash, 'complete -p "$PWD/prefix1/sbin/cmd2"', want_output=True
+            )
+            assert "/prefix1/sbin/cmd2" in output
 
     def test_cmd_path_1(self, bash):
+        assert_bash_exec(bash, "complete -r cmd1 || :", want_output=None)
         output = assert_bash_exec(
             bash, "__load_completion prefix1/bin/cmd1", want_output=True
         )
         assert output.strip() == "cmd1: sourced from prefix1"
+        output = assert_bash_exec(
+            bash, 'complete -p "$PWD/prefix1/bin/cmd1"', want_output=True
+        )
+        assert "/prefix1/bin/cmd1" in output
+        assert_bash_exec(bash, "! complete -p cmd1", want_output=None)
         output = assert_bash_exec(
             bash, "__load_completion prefix1/sbin/cmd2", want_output=True
         )