fix: load from more new style completion dirs

[scop]

This might not be spot on off the bat, but it restores some of the way we used to load completions from.

In particular, before this, completions-core and completions-fallback from the bash_completion location were not attempted, which is a regression at least for my workflow: I rely on being able to modify files in my bash-completion working dir and have them take precendence over any system installed ones.

[scop]

The result of this is that we end up loading completions without the .bash suffix from more dirs we did before this change. We may want to tighten that up some.

Could check if the dir ends with -core or -fallback and load only .bash for them. That isn't 100% correct in the case if someone has for whatever reason configured their BASH_COMPLETION_USER_DIR to have one of those suffixes (we'd want to load both with and without .bash from there for backwards compatibility).

[akinomyoga]

I agree to check the suffix -{core,fallback} (probably inside _comp_load__visit_file).

[akinomyoga]

Thank you for creating the PR! This is exactly the issue that I described in #1557 (comment).

When using the new version, I also noticed a search-order problem of _comp_load when the system bash-completion < 2.18 and the user's bash-completion >= 2.18 coexist. I'll later describe it. I would like to discuss it and fix issues of _comp_load including it.

I was naively thinking of swapping the directory precedence of 2) and 3), and inserting the search for completions-core before 4), but I haven't yet considered it deeply enough.

[akinomyoga]

BASH_COMPLETION_USER_DIR is supposed to contain a path like ~/.local/share/bash-completion, so we need to suffix /completion to them too. I think we can use _comp_compgen. For example, something like this: _comp_compgen -Rv dirs -F : -- -W '$BASH_COMPLETION_USER_DIR' -S /completions, though I haven't tested this carefully.

[akinomyoga]

Should we search the system completions-{core,fallback} after none were found in the in-tree completions-{core,fallback} (i.e., $_comp__base_directory/completions-{core,fallback} in the tree of the current bash_completion)? I think all default/fallback completions should already be found in the in-tree completions-{core,fallback}. Are there cases where the completion file is not found in the in-tree completions-{core,fallback} but found in the system completions-{core,fallback}?

I also think loading external completions-{core,fallback} would cause the compatibility problem with the internal API. We've been assuming that we can use the latest internal APIs defined in bash_completion inside the in-tree completions of the same version. When the in-tree completions-{core,fallback} doesn't contain the completion for a command while the system completions-{core,fallback} contains one, I think that implies the system bash-completion is newer than the current bash-completion. In this case, we should avoid loading the system completions-{core,fallback} because it is likely to contain uses of new internal APIs.

In short, I think the completions in completions-{core,fallback} are tightly bound, in the sense of internal API, to the exact version of bash_completion in the same tree. It's not safe to load completions-{core,fallback} outside the tree.

[akinomyoga]

For the same reason, I think we don't want to load completions-{core,fallback} in the bin directories.

Also, as I mentioned above, I would like to suggest exchanging the ordering of 2) and 3).

[akinomyoga]

Another issue is that when, the current bash_completion's tree ($_comp__base_directory) is different from the system's one (/usr/share/bash-completion), with the suggested search ordering, external completions installed in the system directory /usr/share/bash-completion/completions/<cmd>.bash will be masked by our completions in completions-{core,fallback}. This implies that the way to override bash-completion's in-tree completion is missing, even if the upstream decides to start hosting the completion file.

• I think completions-fallback should at least be checked at the very last step as before this PR.

• We should also be aware of the version time lag of the move of the file completions-{core => fallback}; even if we move a completion file to a fallback directory on request from the upstream, our in-tree completion continues to take precedence until the bash-completion version in the market is entirely replaced (which takes several to ten years).

  For this reason, I think the search ordering that we currently discuss in this PR should be a temporary one; when new bash-completion versions (having in-tree completions in completions-{core,fallback}) are available in distributions, I think we should move back the search order of the in-tree completions-core after 1)..4) directories.

[akinomyoga]

• For this reason, I think the search ordering that we currently discuss in this PR should be a temporary one; when new bash-completion versions (having in-tree completions in completions-{core,fallback}) are available in distributions, I think we should move back the search order of the in-tree completions-core after 1)..4) directories.

Or, maybe we can keep the current master, and set something like XDG_DATA_DIRS=/dev/null for testing purposes.

Or, another possibility is to search i) 1-4 with .bash, ii) in-tree completions-core, iii) 1-4 without suffix, iv) completions-fallback. The biggest problem currently is that completions in /usr/share/bash-completion/completions/<cmd> provided by the system bash-completion package have higher precedence than the in-tree completions. In the current search ordering, we need to wait until distributed versions of bash-completion are updated to be the ones with the new directory structure. Loading of old completions provided by the system bash-completion package can be avoided by checking the .bash suffix. That's the idea.

[akinomyoga]

Or, another possibility is to search i) 1-4 with .bash, ii) in-tree completions-core, iii) 1-4 without suffix, iv) completions-fallback.

Currently, I like this idea, though I need to think about it more later. It seems to solve the main issues cleanly with the minimum change.

diff --git a/bash_completion b/bash_completion
index e67cc3d51..d9c1d1cce 100644
--- a/bash_completion
+++ b/bash_completion
@@ -3508,15 +3508,17 @@ _comp_load()
     shift
     local -a source_args=("$@")

-    local i
-    for i in "${!dirs[@]}"; do
-        dir=${dirs[i]}/completions
-        [[ -d $dir ]] || continue
-        for compfile in "$cmdname.bash" "$cmdname"; do
-            _comp_load__visit_file "$dir/$compfile" && return 0
-        done
+    local dir
+    for dir in "${dirs[@]}"; do
+        _comp_load__visit_file "$dir/completions/$cmdname.bash" && return 0
     done
+
     _comp_load__visit_file "$_comp__base_directory/completions-core/$cmdname.bash" && return 0
+
+    for dir in "${dirs[@]}"; do
+        _comp_load__visit_file "$dir/completions/$cmdname" && return 0
+    done
+
     _comp_load__visit_file "$_comp__base_directory/completions-fallback/$cmdname.bash" && return 0

     # Look up simple "xspec" completions