Option to only complete flags when - has been typed

[mrliptontea]

First of all, thank you for your excellent projects, bashly and completely. They are fantastic and certainly made creating Bash tools more bearable.

Seeing the improvements made to flag autocompletion from DannyBen/bashly#224, I wonder if it would be possible, maybe as an opt-in setting, to make the completion only output flags when the cursor has stopped at -, and output no flags otherwise.

I think this behaviour is pretty common and used in many tools' completions. In essence:

# complete flags:
mytool -<TAB>
mytool --<TAB>
# complete other stuff, i.e. files, folders, etc.
mytool <TAB>

What do you think?

On a separate note, normally shellcheck complains about read without -r, I'm unsure if it should be included here?
See https://www.shellcheck.net/wiki/SC2162

[DannyBen]

I am happy you find them useful, thanks for sharing.

As for the "no flag completion unless the cursor is at -" - I don't hate the idea, assuming it can be implemented without influencing the user interface (config syntax).

Just to be sure we are on the same page:

Given this completely.yaml

mygit:
- -h
- -v
- --help
- --version
- pull
- push
- status
- commit

running this commant:

$ completely test "mygit "

currently outputs:

-h
-v
--help
--version
pull
push
status
commit

and you want it to output just:

pull
push
status
commit

yes?

And as for the shellcheck - I would love to make the output script shellcheck compliant.
When running shellcheck on the generated completely.bash script, I in fact get two issues:

$ shellcheck completely.bash

In completely.bash line 9:
  local compline="${COMP_WORDS[@]:1:$COMP_CWORD-1}"
                 ^-- SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In completely.bash line 13:
      while read; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "-h -v --help --version pull push status commit" -- "$cur" )
            ^--^ SC2162 (info): read without -r will mangle backslashes.

and when I manually change the script with these two changes:

 _mygit_completions() {
   local cur=${COMP_WORDS[COMP_CWORD]}
-  local compline="${COMP_WORDS[@]:1:$COMP_CWORD-1}"
+  local compline="${COMP_WORDS[*]:1:$COMP_CWORD-1}"

   case "$compline" in
     *)
-      while read; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "-h -v --help --version pull push status commit" -- "$cur" )
+      while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "-h -v --help --version pull push status commit" -- "$cur" )
       ;;

   esac

shellcheck is satisfied, and it seems to be working just the same. If this can be confirmed as non breaking, I can modify the template.

[mrliptontea]

$ completely test "mygit "

currently outputs:

-h
-v
--help
--version
pull
push
status
commit

and you want it to output just:

pull
push
status
commit

yes?

Exactly, and running:

$ completely test "mygit -"

would output:

-h
-v
--help
--version

This is how I find completions to work in general, e.g. for git, grep, sed, kubectl, docker, etc.

---

shellcheck is satisfied, and it seems to be working just the same. If this can be confirmed as non breaking, I can modify the template.

Yeah, on this one I always listened to shellcheck, this demonstrates the issue:

# with -r
$ while read -r line; do printf '%s\n' "$line"; done < <(echo 'so much fu\\n')
so much fu\n # correct, backslash preserved literally, as it was in the original output

# without -r
$ while read line; do printf '%s\n' "$line"; done < <(echo $'so much fu\\n')
so much fu
# <- incorrect, we're on a new line

[DannyBen]

My "fix" for the other shellcheck issue (https://www.shellcheck.net/wiki/SC2124) is wrong.
If we are going to fix shellcheck issues, I would like to fix them both. If you have a solution for it, I would love to know.

As for the implementation of the primary topic of this issue - I will need to take a deeper look if this can be easily implemented. I am not sure - due to the fact that at this point, the generated script does not care at all about the cursor position - it lets compgen worry about it.

[DannyBen]

If you can take a look at #33 - I will merge it, so we have at least 1/3 of the issues solved.

[mrliptontea]

If we are going to fix shellcheck issues, I would like to fix them both. If you have a solution for it, I would love to know.

I think #33 does it.

As for the implementation of the primary topic of this issue - I will need to take a deeper look if this can be easily implemented. I am not sure - due to the fact that at this point, the generated script does not care at all about the cursor position - it lets compgen worry about it.

The script doesn't need to worry about the cursor position, compgen does, but it would need to know what current word starts with. I was thinking maybe it could look like this:

if [[ $cur == -* ]] ; then
  case "$comp_line" in
    # complete flags
  esac
else
  case "$comp_line" in
    # complete other stuff
  esac
fi

And then the main problem is splitting specific cases and compgen arguments accordingly.

[DannyBen]

I think #33 does it.

No. It only solves the read -r alert. I am using shellcheck 0.8.0.
This error still exists:

  local compline="${COMP_WORDS[@]:1:$COMP_CWORD-1}"
                 ^-- SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.

The script doesn't need to worry about the cursor position

Yeah. I said cursor, but meant current word. Completely currently discards it, it will need to use it.
Notice how from a single completion line per pattern, we will now have 9 lines of code for each pattern. Not sure this is acceptable in my book.

[mrliptontea]

No. It only solves the read -r alert. I am using shellcheck 0.8.0. This error still exists:

  local compline="${COMP_WORDS[@]:1:$COMP_CWORD-1}"
                 ^-- SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.

Oh, I completely missed this. So the right side produces an array. Since there's a difference in behaviour in different shells (in zsh ${array[*]:1} concatenates first, then removes the first character), maybe we just need to assign modified array to a temporary variable, and then concatenate:

local comptemp=("${COMP_WORDS[@]:1:$COMP_CWORD-1}")
local compline="${comptemp[*]}"

That would satisfy shellcheck.

---

Yeah. I said cursor, but meant current word. Completely currently discards it, it will need to use it. Notice how from a single completion line per pattern, we will now have 9 lines of code for each pattern. Not sure this is acceptable in my book.

I'm not sure what you mean, $cur is there, that's what we need. As for the extra lines, yes, there will be some but not that many:

Assuming config:

mygit:
- --help
- --version
- push

mygit push:
- --help
- --force

it would produce:

if [[ $cur == -* ]] ; then
  case "$compline" in
    'push'*)
      while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "--help --force" -- "$cur" )
      ;;
    *)
      while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "--help --version" -- "$cur" )
      ;;
  esac
else
  case "$compline" in
    *)
      while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "push" -- "$cur" )
      ;;
  esac
fi

[mrliptontea]

However, the problem I'm now seeing is what if I had a custom command for completions, like $(mycompletion 2>/dev/null) that could also complete flags, either it would need to be included in both cases, or there would have to be a special syntax when defining completions. I'm thinking e.g. about extensible commands, like:

cli:
- --help
- --version
# this would be included in `compgen -W` if `$cur` *doesn't* start with `-`
- $(mycompleter 2>/dev/null)
# this would be included in `compgen -W` if `$cur` *does* start with `-`
- flags:$(mycompleter 2>/dev/null)

So here flags: is a special prefix denoting when it should be included, then it would need to be discarded, and the rest of the string would be used as compgen -W argument.

This is basically something I'm doing right now, where I have a wrapper around kubectl in bashly that calls kubectl with extra arguments but I still want it to complete all kubectl options. And it works, except I also get bashly's flags, e.g.:

$ mykubectl get pod <TAB>
pod1    pod2    --help    --version

instead of just:

$ mykubectl get pod <TAB>
pod1    pod2

Hence I raised this 🙂

[DannyBen]

The second shellcheck fix is ready in #34 - if you can review it will be nice (also added a short spec to ensure the generated script is always shellcheck compliant).

As for the flag completion issue - maybe we should approach it from a simpler angel. How about:

# old line
while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "--help --verbose ... " -- "$cur" )

# new line
while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W get_word_string -- "$cur" )

# and the function pseudo code
get_word_string() {
  # remove string words if they start with "-" and $cur is not starting with "-"
  return string
}

This makes it so we move the logic to a "post processing" step, and therefore it will support custom commands just the same.

Thoughts?

---

$ mykubectl get pod <TAB>
pod1    pod2    --help    --version

P.S.. And now when I see it, I agree it is terribly annoying.

[mrliptontea]

I like the post processing wrapper idea!

[DannyBen]

I like the post processing wrapper idea!

I'll play with it then.

[DannyBen]

Does this seem right to you as a basis?

#!/usr/bin/env bash
_filter() {
  local words="$1"
  local cur="a"  # this will be taken from the actual input
  local result=()

  if [[ "${cur:0:1}" == "-" ]]; then
    echo "$words"
  
  else
    for word in $words; do
      [[ "${word:0:1}" != "-" ]] && result+=("$word")
    done

    echo "${result[*]}"

  fi
}

( compgen -W "$(_filter "--help --version status commit")" -- "$cur" )

EDIT: I replaced the function so it handles a string and not an array, so it it easier to send it from within a compgen call.

[DannyBen]

Ok - #35 includes the new implementation. There is no configuration to switch between the old and new completions.

This is quite a change and has a breaking potential - so it needs to be thoroughly reviewed.
Tests are passing of course.

One thing left to do, is document this.

[DannyBen]

Completely 0.5.0 released to rubygems and dockerhub with this change.
Thanks for raising this issue.

(bashly will be released shortly)

[mrliptontea]

It works beautifully, thank you for doing it so quickly as well ❤️

[DannyBen]

Excellent, thanks for confirming and for participating in this fix.
Feel free to open a new issue if you have other ideas for improvements or encounter any problems.