Skip to content

Do not replace already existing completions #2298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 26, 2025
Merged

Do not replace already existing completions #2298

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46033be
FUNCTIONAL: Do not replace already existing completions
convergedtarkus Apr 2, 2025
83a519d
FUNCTIONAL: Replace alias completions on reload
convergedtarkus Apr 3, 2025
c5caae0
Remove old aliases
convergedtarkus Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions completion/available/aliases.completion.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@ function _bash-it-component-completion-callback-on-init-aliases() {
line="${line#alias -- }"
line="${line#alias }"
alias_name="${line%%=*}"

# Skip aliases not added by this script that already have completion functions.
# This allows users to define their own alias completion functions.
# For aliases added by this script, we do want to replace them in case the
# alias getting the completion added has changed.
if complete -p "$alias_name" &> /dev/null; then
# Get the -F argument from the existing completion for this alias.
aliasCommandFunction=$(complete -p "$alias_name" | rev | cut -d " " -f 2 | rev)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this variable be localized?

Also, I'm not sure if one can safely assume that the second last word of complete -p <alias> would be the argument of the -F option. There can be completion settings with -C <cmd> or ones without neither -F nor -C.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are absolutely right. Adding it as a local var in my next PR.

@convergedtarkus you said you fully tested this, right? Got any input regarding the complete issue raised by Koichi-san?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on making the variable local.

As for the logic, I think the logic is safe given that the scope is isolated.

This logic checks if the current alias being analyzed has an existing completion. If it does, this command tries to extract the name of the function associated with the completion. If the extracted name is anything but the namespace used for the aliases added by this script, that alias is skipped over because it has an existing completion function that was not added by this script. So the script only cares about aliases with existing completion functions added by this script.

Since this code is only looking for the completion commands added by this script, we can know they are created in a predictable manner. Anything else just ends up being skipped.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation. I haven't carefully looked at the code around this line. Checking prefix _${namespace}:: (where namespace is alias_completion) should usually be sufficient. You can mark this conversation as marked (I'm not the PR author or a maintainer, so I don't have access to the resolve button).

# Check if aliasCommandFunction starts with our namespace.
if [[ "$aliasCommandFunction" != "_${namespace}::"* ]]; then
continue
fi

# Remove existing completion. It will be replaced by the new one. We need to
# delete it in case the new alias does not support having completion added.
complete -r "$alias_name"
fi

alias_defn="${line#*=\'}" # alias definition
alias_defn="${alias_defn%\'}"
alias_cmd="${alias_defn%%[[:space:]]*}" # first word of alias
Expand Down Expand Up @@ -71,15 +89,20 @@ function _bash-it-component-completion-callback-on-init-aliases() {
fi
new_completion="$(complete -p "$alias_cmd" 2> /dev/null)"

# create a wrapper inserting the alias arguments if any
if [[ -n $alias_args ]]; then
compl_func="${new_completion/#* -F /}"
compl_func="${compl_func%% *}"
# avoid recursive call loops by ignoring our own functions
if [[ "${compl_func#_"$namespace"::}" == "$compl_func" ]]; then
compl_wrapper="_${namespace}::${alias_name}"
compl_func="${new_completion/#* -F /}"
compl_func="${compl_func%% *}"
# avoid recursive call loops by ignoring our own functions
if [[ "${compl_func#_"$namespace"::}" == "$compl_func" ]]; then
compl_wrapper="_${namespace}::${alias_name}"

# Create a wrapper function for the alias
if [[ -z $alias_args ]]; then
# Create a wrapper without arguments.
# This allows identifying the completions added by this script on reload.
echo "function $compl_wrapper {
$compl_func \"\$@\"
}" >> "$tmp_file"
else
# Create a wrapper inserting the alias arguments
# The use of printf on alias_arg_words is needed to ensure each element of
# the array is quoted. E.X. (one two three) -> ('one' 'two' 'three')
echo "function $compl_wrapper {
Expand All @@ -99,8 +122,8 @@ function _bash-it-component-completion-callback-on-init-aliases() {
(( COMP_POINT += \${#COMP_LINE} ))
\"$compl_func\" \"$alias_cmd\" \"\$compl_word\" \"\$prec_word\"
}" >> "$tmp_file"
new_completion="${new_completion/ -F $compl_func / -F $compl_wrapper }"
fi
new_completion="${new_completion/ -F $compl_func / -F $compl_wrapper }"
fi

# replace completion trigger by alias
Expand Down
Loading