[PATCH bash ordered autocomplete] Order the autocompletion in bash

J Rt jean.rblt at gmail.com
Sun Mar 29 23:31:07 CEST 2020


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 <reedwade at misterbanal.net> 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 <rephorm at rephorm.com>. 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/}"
> > -%<-


More information about the Password-Store mailing list