Many thanks for the detailed code review. So do you want me to create a new commit on top of the old one fixing the things you mention, and send it again? :) Or did you take care of pushing something you like better already? I looked on the git hosting side, I could not find this branch. If you do not mind, and if ok for you, maybe it is simplest that you do the changes you like best.
Sorry for the noobness, I am quite familiar with the GitHub method for all of this, but not with your self hosted solution :( On Sun, Mar 29, 2020 at 6:49 PM Reed Wade <[email protected]> wrote: > > > diff --git a/src/completion/pass.bash-completion > > b/src/completion/pass.bash-completion > > index 95d3e1e..95dcb40 100644 > > --- a/src/completion/pass.bash-completion > > +++ b/src/completion/pass.bash-completion > > @@ -4,7 +4,26 @@ > > # Brian Mattern <[email protected]>. All Rights Reserved. > > # This file is licensed under the GPLv2+. Please see COPYING for more > > information. > > > > +_sort_entries_string () { > > + # local sorted_list=$(echo $1 | xargs -n1 | sort | xargs) > > Is it forgotten? > > > + local sorted_list=$(echo $1 | tr ' ' '\n' | sort | tr '\n' ' ') > > + echo $sorted_list > > Just use `echo $1 | tr ' ' '\n' | sort | tr '\n' ' '` directly and > remove the local variable. > > > +} > > + > > +_setup_tmp_compreply () { > > + TMP_COMPREPLY=() > > +} > > IMHO, this function is not really usefull. We could rafactorize this later > if the setup is more complicated than `=()`. > > Plus, I think you should not use this kind of global variable but simply > use a "entries" local var in the different functions then append the > sorted version. I recommend you to use a simple string variable. You'll > feel it easier to sort. COMPREPLY still need to be an array ofc cause of > bash .. ^^ > > > + > > +_append_tmp_compreply () { > > + # sort the TMP_COMPREPLY array > > + IFS=$'\n' TMP_COMPREPLY=($(sort <<<"${TMP_COMPREPLY[*]}")); unset IFS > > + # append the tmp array to the COMPREPLY > > + COMPREPLY+=( "${TMP_COMPREPLY[@]}" ) > > +} > > Give this method the tmp_compreply words variable as first argument. > > Something as: > > ```diff > - _append_tmp_compreply () { > - # sort the TMP_COMPREPLY array > - IFS=$'\n' TMP_COMPREPLY=($(sort <<<"${TMP_COMPREPLY[*]}")); unset IFS > - # append the tmp array to the COMPREPLY > - COMPREPLY+=( "${TMP_COMPREPLY[@]}" ) > - } > + _append_to_compreply () { > + for word in "$(_sort_entries_string "$1")"; do > + COMPREPLY+="$word" > + done > + } > ``` > > This example shows you that commentaries are not mandatory if the code > is simple! > > Plus, be carreful to use the good indentation way. In this script, we use > tabulation instead of spaces. > > This changes got impact in the 3 differents `_pass_complete_blabla` > methods. I let you to find the better way to change them. > > > + > > _pass_complete_entries () { > > + _setup_tmp_compreply > > + > > local prefix="${PASSWORD_STORE_DIR:-$HOME/.password-store/}" > > -%<-
