feat: support the startup{,-core} directory
#1558
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3584,7 +3584,60 @@ _comp__init_collect_startup_configs() | |
| local base_path=${1:-${BASH_SOURCE[1]}} | ||
| _comp__init_startup_configs=() | ||
|
|
||
| # list startup directories | ||
| local -a startup_dirs=() | ||
| local paths | ||
| _comp_split -F : paths "${BASH_COMPLETION_USER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/bash-completion}" && | ||
| startup_dirs+=("${paths[@]/%//startup}") | ||
| _comp_split -F : paths "${XDG_DATA_DIRS:-/usr/local/share:/usr/share}" && | ||
| startup_dirs+=("${paths[@]/%//bash-completion/startup}") | ||
| startup_dirs+=("$_comp__base_directory/startup") | ||
| startup_dirs+=("$_comp__base_directory/startup-core") | ||
|
|
||
| # collect startup files | ||
| local -A startup_visited=() | ||
| for startup_dir in "${startup_dirs[@]}"; do | ||
| [[ -d $startup_dir && -r $startup_dir && -x $startup_dir ]] || continue | ||
|
|
||
| local -a startup_files=() | ||
| _comp_expand_glob startup_files "$startup_dir/*.bash" || continue | ||
|
|
||
| local -a selected_startup_files=() | ||
| local startup_dir | ||
| for startup_file in "${startup_files[@]}"; do | ||
| [[ ! -d $startup_file && -r $startup_file ]] || continue | ||
|
|
||
| # Disable the startup files with the same name as one of already | ||
| # selected startup files. This allows users to override a startup | ||
| # file with another file with the same name placed in an earlier | ||
| # directory in "${startup_files[@]}". | ||
| local name=${startup_file##*/?([0-9][0-9][0-9]_)} | ||
|
Collaborator
There was a problem hiding this comment. Why is this processing needed?
Collaborator
Author
There was a problem hiding this comment. I think this comment discusses the same thing as the following cover comment, so let me answer together in this sub-thread.
The original motivation that I thought of this mechanism is #1399. Although the OP of #1399 didn't explain or show a reference to any technical details that they tried to use to reason the PR, I found the UAPI spec and other materials myself. They explain the usage of Meanwhile, I've been thinking it is very annoying that some distribution packages put in This also becomes an issue in a shared system, for which I do not have the administrator privilege. If there are any files in Even if we forget about those arguments, when different versions of the same application install the same name of startup completion files at different paths, such as
If you mean by "this" the entire processing described by the code comment, I think I answered it above. If you mean by "this" particularly the stripping of
Collaborator
Author
There was a problem hiding this comment. Maybe the name "eagerly loaded settings" is not a good description for startup files that can be overridden by users?
Collaborator
Author
There was a problem hiding this comment. I realized I haven't considered users' directories in looking up the users' startup files. I added the support for it in commit a45e2ab. I'll squash this commit into the first commit later.
Collaborator
Author
There was a problem hiding this comment. I also included the description of this behavior in fc6787f.
Collaborator
Author
There was a problem hiding this comment. Thank you for pointing out the existence of the configuration variable, |
||
| [[ ${startup_visited[$name]-} ]] && continue | ||
| startup_visited[$name]=set | ||
|
|
||
| selected_startup_files+=("$startup_file") | ||
| done | ||
| ((${#selected_startup_files[@]})) || continue | ||
|
|
||
| # Note: bash < 4.4 has a bug that all elements are connected with | ||
| # ${a[@]+"${a[@]}"} when IFS does not contain whitespace. | ||
| local IFS=$' \t\n' | ||
| # Prepend the selected startup files because we want to source the | ||
| # system startup files earlier. | ||
| _comp__init_startup_configs=( | ||
| "${selected_startup_files[@]}" | ||
|
|
||
| # Note: bash < 4.3 has a bug that "${a[@]}" fails for "set -u" when | ||
| # an array "a" exists but has no elements. We use | ||
| # ${a[@]+"${a[@]}"} as a workaround. Similar patterns are also | ||
| # used in completions/*.bash, 000_bash_completion_compat.bash, and | ||
| # test_unit_compgen.py. They need to be updated when the minimum | ||
| # required Bash version is lifted up. | ||
| ${_comp__init_startup_configs[@]+"${_comp__init_startup_configs[@]}"} | ||
|
Collaborator
There was a problem hiding this comment. Side note: I see we have this pattern in other places, but I don't understand why it's needed, doesn't just doing work?
Collaborator
Author
There was a problem hiding this comment. Bash 4.2 has a bug that
Owner
There was a problem hiding this comment. Just a random off-context comment, as I haven't been following this MR yet, ref #1479 (reply in thread): I think we should drop support for bash < 5.0 (or at least < 4.3) for the next release.
Collaborator
Author
There was a problem hiding this comment. So they can finally be removed, but I believe it should be done in a separate PR for consistency (and for later research in case something happens). In the first place, the code doesn't pass the CI tests without the workaround in the current setup of the test environment.
Collaborator
Author
There was a problem hiding this comment. I added a clarification in I also considered adding the same comments to other cases, but I gave it up because there seem to be many, and I didn't feel that we needed to bother with what would soon be removed for the version requirement change. There have been 13 cases, and none of them had comments: ./bash_completion:975: _comp_compgen -F "$_ifs" -U input -- ${_compgen_options[@]+"${_compgen_options[@]}"} -W '$input'
./bash_completion:1101: local "$2" "$3" "$4" && _comp_upvars -a"${#words[@]}" "$2" ${words[@]+"${words[@]}"} \
./bash_completion:1379: _comp_compgen -U toks set ${toks[@]+"${toks[@]}"}
./bash_completion:2657: _comp_compgen_known_hosts ${options[@]+"${options[@]}"} -- "$cur"
./bash_completion:3413: elif . "$compfile" "$cmd" ${source_args[@]+"${source_args[@]}"}; then
./completions-core/gdb.bash:32: find ${path_array[@]+"${path_array[@]}"} . -name . -o \
./completions-core/ri.bash:23: "$(ri ${classes[@]+"${classes[@]}"} 2>/dev/null | ruby -ane \
./completions-core/ri.bash:31: "$(ruby -W0 "$ri_path" ${classes[@]+"${classes[@]}"} 2>/dev/null | ruby -ane \
./completions-core/strace.bash:60: '${syscalls[@]+"${!syscalls[@]}"} file process
./completions-core/update-rc.d.bash:20: _comp_compgen -- -W '"${options[@]}" ${services[@]+"${services[@]}"}' \
./startup-core/000_bash_completion_compat.bash:338: args=(-c "$REPLY" ${opt[@]+"${opt[@]}"})
./startup-core/000_bash_completion_compat.bash:362: args=(-c "$REPLY" ${opt[@]+"${opt[@]}"})
./test/t/unit/test_unit_compgen.py:22: '_comp__test_words() { local -a input=("${@:1:$#-1}"); _comp__test_compgen -c "${@:$#}" -- -W \'${input[@]+"${input[@]}"}\'; }', |
||
| ) | ||
| done | ||
|
|
||
| # collect compat completion directory definitions | ||
| local -a compat_dirs=() | ||
| local compat_dir | ||
| if [[ ${BASH_COMPLETION_COMPAT_DIR-} ]]; then | ||
|
|
@@ -3599,25 +3652,33 @@ _comp__init_collect_startup_configs() | |
| # functions. | ||
| if [[ $_comp__base_directory == */share/bash-completion ]]; then | ||
| compat_dir=${_comp__base_directory%/share/bash-completion}/etc/bash_completion.d | ||
| [[ ${compat_dirs[0]} == "$compat_dir" ]] || | ||
| compat_dirs+=("$compat_dir") | ||
| fi | ||
| fi | ||
| local -A compat_visited=() | ||
| for compat_dir in "${compat_dirs[@]}"; do | ||
| [[ -d $compat_dir && -r $compat_dir && -x $compat_dir ]] || continue | ||
| local compat_files | ||
| _comp_expand_glob compat_files '"$compat_dir"/*' || continue | ||
| local compat_file | ||
| for compat_file in "${compat_files[@]}"; do | ||
| [[ ${compat_file##*/} != @($_comp_backup_glob|Makefile*|${BASH_COMPLETION_COMPAT_IGNORE-}) && | ||
| -f $compat_file && -r $compat_file ]] || continue | ||
|
|
||
| # Disable the compat files with the same name as one of already | ||
| # selected ones. This allows users to override a compat file with | ||
| # another file with the same name placed in an earlier directory in | ||
| # "${compat_dirs[@]}". | ||
| local name=${compat_file##*/} | ||
| [[ ${compat_visited[$name]-} ]] && continue | ||
| compat_visited[$name]=set | ||
|
|
||
| _comp__init_startup_configs+=("$compat_file") | ||
| done | ||
| done | ||
|
|
||
| # collect the user completion file | ||
| # | ||
| # Remark: We explicitly check that $user_completion is not '/dev/null' | ||
| # since /dev/null may be a regular file in broken systems and can contain | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| startupcoredir = $(datadir)/$(PACKAGE)/startup-core | ||
| startupcore_DATA = 000_bash_completion_compat.bash | ||
|
|
||
| EXTRA_DIST = $(startupcore_DATA) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| startupdir = $(datadir)/$(PACKAGE)/startup | ||
| startup_DATA = README.md | ||
|
|
||
| EXTRA_DIST = $(startup_DATA) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # External startup directory - bash-completion | ||
|
|
||
| This directory `startup` is the place to put _external startup scripts_ | ||
| for initializing [bash-completion](https://github.com/scop/bash-completion). | ||
|
|
||
| The `bash-completion` framework has been providing `/etc/bash_completion.d` as | ||
| a directory to put scripts that are eagerly loaded on the initialization stage | ||
| of `bash-completion`. However, the historical purpose of | ||
| `/etc/bash_completion.d` has been the place to put completion files for | ||
| `bash-completion < 2.0`, which did not support dynamic loading through | ||
| `complete -D` (only available in `bash >= 4.1`). This original usage was | ||
| already deprecated. Apart from that, there are scripts that need to be loaded | ||
| on the initialization stage of `bash-completion`, such as | ||
| `/etc/bash_completion.d/fzf` and `/etc/bash_completion.d/_python-argcomplete`, | ||
| which want to set up a custom default completion with `complete -D`. | ||
|
|
||
| In `bash-completion >= 2.18`, we started to provide the `startup` directory as | ||
| a dedicated directory for such scripts that need to be loaded on the | ||
| initialization stage. | ||
|
|
||
| * Eagerly loaded scripts should be placed in this directory instead of | ||
| `/etc/bash_completion.d`. | ||
| * External completion files of API v1 should be updated to use API v2 or v3 and | ||
| should be moved to `completions/<cmd>.bash`. Although we recommend to update | ||
| the completion file to use newer API, the completion file relying on API v1 | ||
| should continue to be placed in `/etc/bash_completion.d`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to create
${sysconfdir}/bash_completion.dThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we? #1399 seemed to want us not to create anything in
/etc. I thought the projects that want to put any file in/etc/bash-completion.dcan create a directory themselves, but does it actually fail in a typical setup of autoconf? I thought it would automatically create a directory. Is there another reason that we need to create it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When creating a package, one would do
make installinto a clean directory. So I guessmake installis rather required to work without any existing directory structures.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's a good point, any other packages that install there will create the directory there themselves, especially since they can be installed even if bash-completion isn't.
My only worry is for users following guides that tell them to put the completions in
/etc/bash-completion.d/. Maybe this calls for another README 🙃I think creating the directory will be fine from the perspective of the UAPI concerns, since we're not dependent anymore on any files in that directory for a fully working system. Maybe someone from the original PR wants to chime in? @septatrix @bluca
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I already removed that part with
6a78aa3. Is there still a place to modify?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean guides outside of the repo, for example this stack overflow question: https://askubuntu.com/questions/68175/how-to-create-script-with-auto-complete, or the cobra13 docs. Maybe adding a bit more friction for doing the "wrong thing" does make sense though.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It can sometimes take a long time to update the community's knowledge. Until recently, there have been many articles recommending the use of the old-style command substitutions
`...`that weredeprecateddiscouraged by the standard about 30 years ago.