Merge pull request #696 from akinomyoga/extend-search-paths

Author: scop

README.md

@@ -112,11 +112,12 @@ A. No. Use `M-/` to (in the words of the bash man page) attempt file
 
 A. Install a local completion of your own appropriately for the desired
    command, and it will take precedence over the one shipped by us. See the
-   next answer for details where to install it, if you are doing it on per
-   user basis. If you want to do it system wide, you can install eagerly
-   loaded files in `compatdir` (see a couple of questions further down for
-   more info) and install a completion for the commands to override our
-   completion for in them.
+   next answer for details where to install it, if you are doing it on per user
+   basis. If you want to do it system wide, you can install eagerly loaded
+   files in `compatdir` (see a couple of questions further down for more
+   info. To get the path of `compatdir` for the current system, the output of
+   `pkg-config bash-completion --variable compatdir` can be used) and install a
+   completion for the commands to override our completion for in them.
 
    If you want to use bash's default completion instead of one of ours,
    something like this should work (where `$cmd` is the command to override
@@ -138,9 +139,14 @@ A. Put them in the `completions` subdir of `$BASH_COMPLETION_USER_DIR`
    completion code for this package. Where should I put it to be sure
    that interactive bash shells will find it and source it?**
 
-A. Install it in one of the directories pointed to by
-   bash-completion's `pkgconfig` file variables. There are two
-   alternatives:
+A. [ Disclaimer: Here, how to make the completion code visible to
+   bash-completion is explained.  We do not require always making the
+   completion code visible to bash-completion.  In what condition the
+   completion code is installed should be determined at the author/maintainers'
+   own discretion. ]
+
+   Install it in one of the directories pointed to by bash-completion's
+   `pkgconfig` file variables. There are two alternatives:
 
    - The recommended directory is `completionsdir`, which you can get with
      `pkg-config --variable=completionsdir bash-completion`. From this
@@ -169,7 +175,7 @@ A. Install it in one of the directories pointed to by
 
    ```makefile
    bashcompdir = @bashcompdir@
-   dist_bashcomp_DATA = # completion files go here
+   dist_bashcomp_DATA = your-completion-file # completion files go here
    ```
 
    For cmake we ship the `bash-completion-config.cmake` and
@@ -190,6 +196,28 @@ A. Install it in one of the directories pointed to by
      ${BASH_COMPLETION_COMPLETIONSDIR})
    ```
 
+   In bash-completion >= 2.12, we search the data directory of
+   `bash-completion` under the installation prefix where the target command is
+   installed.  When one can assume that the version of the target
+   bash-completion is 2.12 or higher, the completion script can actually be
+   installed to `$PREFIX/share/bash-completion/completions/` under the same
+   installation prefix as the target program installed under `$PREFIX/bin/` or
+   `$PREFIX/sbin/`.  For the detailed search order, see also "Q. What is the
+   search order for the completion file of each target command?" below.
+
+   Example for `Makefile.am`:
+
+   ```makefile
+   bashcompdir = $(datarootdir)/bash-completion/completions
+   dist_bashcomp_DATA = your-completion-file
+   ```
+
+   Example for `CMakeLists.txt`:
+
+   ```cmake
+   install(FILES your-completion-file DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/bash-completion/completions")
+   ```
+
 **Q. When completing on a symlink to a directory, bash does not append
    the trailing `/` and I have to hit <kbd>&lt;Tab></kbd> again.
    I don't like this.**
@@ -299,3 +327,28 @@ A. Absolutely not. zsh has an extremely sophisticated completion system
    that offers many features absent from the bash implementation. Its
    users often cannot resist pointing this out. More information can
    be found at <https://www.zsh.org/>.
+
+**Q. What is the search order for the completion file of each target command?**
+
+A. The completion files of commands are looked up by the shell function
+  `__load_completion`.  Here, the search order in bash-completion >= 2.12 is
+  explained.
+
+  1. `BASH_COMPLETION_USER_DIR`. The subdirectory `completions` of each paths
+     in `BASH_COMPLETION_USER_DIR` separated by colons is considered for a
+     completion directory.
+  2. The location of the main `bash_completion` file. The subdirectory
+     `completions` in the same directory as `bash_completion` is considered.
+  3. The location of the target command.  When the real location of the command
+     is in the directory `<prefix>/bin` or `<prefix>/sbin`, the directory
+     `<prefix>/share/bash-completion/completions` is considered.
+  4. `XDG_DATA_DIRS` (or the system directories `/usr/local/share:/usr/share`
+     if empty).  The subdirectory `bash-completion/completions` of each paths
+     in `XDG_DATA_DIRS` separated by colons is considered.
+
+  The completion files of the name `<cmd>` or `<cmd>.bash`, where `<cmd>` is
+  the name of the target command, are searched in the above completion
+  directories in order.  The file that is found first is used.  When no
+  completion file is found in any completion directories in this process, the
+  completion files of the name `_<cmd>` is next searched in the completion
+  directories in order.

bash_completion

@@ -2587,20 +2587,48 @@ complete -F _minimal ''
 
 __load_completion()
 {
-    local -a dirs=(${BASH_COMPLETION_USER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/bash-completion}/completions)
-    local IFS=: dir cmd="${1##*/}" compfile
+    local cmd="${1##*/}" dir compfile
+    local -a paths
     [[ $cmd ]] || return 1
-    for dir in ${XDG_DATA_DIRS:-/usr/local/share:/usr/share}; do
-        dirs+=($dir/bash-completion/completions)
-    done
-    _comp_unlocal IFS
 
+    local -a dirs=()
+
+    # Lookup order:
+    # 1) From BASH_COMPLETION_USER_DIR (e.g. ~/.local/share/bash-completion):
+    # User installed completions.
+    if [[ ${BASH_COMPLETION_USER_DIR-} ]]; then
+        _comp_split -F : paths "$BASH_COMPLETION_USER_DIR"
+        dirs+=("${paths[@]/%//completions}")
+    else
+        dirs=("${XDG_DATA_HOME:-$HOME/.local/share}/bash-completion/completions")
+    fi
+
+    # 2) From the location of bash_completion: Completions relative to the main
+    # script. This is primarily for run-in-place-from-git-clone setups, where
+    # we want to prefer in-tree completions over ones possibly coming with a
+    # system installed bash-completion. (Due to usual install layouts, this
+    # often hits the correct completions in system installations, too.)
     if [[ $BASH_SOURCE == */* ]]; then
         dirs+=("${BASH_SOURCE%/*}/completions")
     else
         dirs+=(./completions)
     fi
 
+    # 3) From bin directories extracted from $(realpath "$cmd") and PATH
+    dir=$(_realcommand "$1")
+    paths=("${dir%/*}")
+    _comp_split -aF : paths "$PATH"
+    for dir in "${paths[@]%/}"; do
+        if [[ -d $dir && $dir == ?*/@(bin|sbin) ]]; then
+            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}
@@ -2611,7 +2639,7 @@ __load_completion()
 
     for dir in "${dirs[@]}"; do
         [[ -d $dir ]] || continue
-        for compfile in "$cmd" "$cmd.bash" "_$cmd"; do
+        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
@@ -2626,6 +2654,22 @@ __load_completion()
         done
     done
 
+    # search deprecated completions "_$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"; 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

test/fixtures/__load_completion/bin/cmd1

@@ -0,0 +1 @@
+../prefix1/bin/cmd1

test/fixtures/__load_completion/bin/cmd2

@@ -0,0 +1 @@
+../prefix1/sbin/cmd2

test/fixtures/__load_completion/prefix1/bin/cmd1

@@ -0,0 +1 @@
+echo cmd1

test/fixtures/__load_completion/prefix1/bin/sh

@@ -0,0 +1 @@
+echo sh

test/fixtures/__load_completion/prefix1/sbin/cmd2

@@ -0,0 +1 @@
+echo cmd2

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

@@ -0,0 +1 @@
+echo 'cmd1: sourced from prefix1'

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

@@ -0,0 +1 @@
+echo 'cmd2: sourced from prefix1'

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

@@ -0,0 +1 @@
+echo 'sh: sourced from prefix1'

test/fixtures/__load_completion/userdir1/completions/cmd1

@@ -0,0 +1 @@
+echo 'cmd1: sourced from userdir1'

test/fixtures/__load_completion/userdir2/completions/cmd2

@@ -0,0 +1 @@
+echo 'cmd2: sourced from userdir2'

test/t/conftest.py

@@ -268,7 +268,7 @@ def bash(request) -> pexpect.spawn:
             # FIXME: Tests shouldn't depend on dimensions, but it's difficult to
             # expect robustly enough for Bash to wrap lines anywhere (e.g. inside
             # MAGIC_MARK).  Increase window width to reduce wrapping.
-            dimensions=(24, 200),
+            dimensions=(24, 240),
             # TODO? codec_errors="replace",
         )
         bash.expect_exact(PS1)

test/t/unit/test_unit_load_completion.py

@@ -0,0 +1,80 @@
+import pytest
+
+from conftest import assert_bash_exec, bash_env_saved
+
+
+@pytest.mark.bashcomp(cmd=None, cwd="__load_completion")
+class TestLoadCompletion:
+    def test_userdir_1(self, bash):
+        with bash_env_saved(bash) as bash_env:
+            bash_env.write_variable(
+                "BASH_COMPLETION_USER_DIR",
+                "$PWD/userdir1:$PWD/userdir2:$BASH_COMPLETION_USER_DIR",
+                quote=False,
+            )
+            bash_env.write_variable(
+                "PATH", "$PWD/prefix1/bin:$PWD/prefix1/sbin", quote=False
+            )
+            output = assert_bash_exec(
+                bash, "__load_completion cmd1", want_output=True
+            )
+            assert output.strip() == "cmd1: sourced from userdir1"
+            output = assert_bash_exec(
+                bash, "__load_completion cmd2", want_output=True
+            )
+            assert output.strip() == "cmd2: sourced from userdir2"
+
+    def test_PATH_1(self, bash):
+        with bash_env_saved(bash) as bash_env:
+            bash_env.write_variable(
+                "PATH", "$PWD/prefix1/bin:$PWD/prefix1/sbin", quote=False
+            )
+            output = assert_bash_exec(
+                bash, "__load_completion cmd1", want_output=True
+            )
+            assert output.strip() == "cmd1: sourced from prefix1"
+            output = assert_bash_exec(
+                bash, "__load_completion cmd2", want_output=True
+            )
+            assert output.strip() == "cmd2: sourced from prefix1"
+
+    def test_cmd_path_1(self, bash):
+        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, "__load_completion prefix1/sbin/cmd2", want_output=True
+        )
+        assert output.strip() == "cmd2: sourced from prefix1"
+        output = assert_bash_exec(
+            bash, "__load_completion bin/cmd1", want_output=True
+        )
+        assert output.strip() == "cmd1: sourced from prefix1"
+        output = assert_bash_exec(
+            bash, "__load_completion bin/cmd2", want_output=True
+        )
+        assert output.strip() == "cmd2: sourced from prefix1"
+
+    def test_cmd_path_2(self, bash):
+        with bash_env_saved(bash) as bash_env:
+            bash_env.write_variable("PATH", "$PWD/bin:$PATH", quote=False)
+            output = assert_bash_exec(
+                bash, "__load_completion cmd1", want_output=True
+            )
+            assert output.strip() == "cmd1: sourced from prefix1"
+            output = assert_bash_exec(
+                bash, "__load_completion cmd2", want_output=True
+            )
+            assert output.strip() == "cmd2: sourced from prefix1"
+
+    def test_cmd_intree_precedence(self, bash):
+        """
+        Test in-tree, i.e. completions/$cmd relative to the main script
+        has precedence over location derived from PATH.
+        """
+        with bash_env_saved(bash) as bash_env:
+            bash_env.write_variable("PATH", "$PWD/prefix1/bin", quote=False)
+            # The in-tree `sh` completion should be loaded here,
+            # and cause no output, unlike our `$PWD/prefix1/bin/sh` canary.
+            assert_bash_exec(bash, "__load_completion sh", want_output=False)