completely icon indicating copy to clipboard operation
completely copied to clipboard

QUESTION: complete a term with a colon / :

Open kronn opened this issue 9 months ago • 2 comments

This project makes me almost completely happy. I want to write a completion for rspec, which is mostly painless. After being finished with my (opinionated) completion, I wanted to also complete the tags, which are tag:value-pairs. I generate the list with a small helper that returns a list that roughly looks like this:

js:false
js:true
sphinx:true
type:controller
type:feature
~js:false
~js:true
~sphinx:true
~type:controller
~type:feature

(naturally, there's more, but this shows the problem well enough)

The tag sphinx:true is easy, since it has the : inside the word. It gets completed in one <TAB>. The negated tags with the ~-prefix are also unproblematic.

For the others, the difference comes after the :. So it is printed but does not allow further completion. I can go back, escape the :, add another character and complete from there.

Let's just say that the ergonomics differ slightly between rsp<TAB>--tag sph<TAB><ENTER> and rsp<TAB>--tag ty<TAB><BACKSPACE>\:c<TAB><ENTER>

And yes, the last <TAB> completes type\:c to type:controller, removing my escape-hack and leaving me with the intended command. However, trying to complete type:c trigger the generic fallback of file-completion for c.

I realize that I do not have a completion-issue, but a shell-escape-issue (as so often when doing things in bash). Do you know of a solution for this? I have the feeling that the _completions_filter-function might be a place for a fix. How this potential fix is the propagated into the config-yaml is as unknown as the fix itself.

This is all I know now. Can you help or provide pointers for a PR I can create?

kronn avatar May 16 '24 16:05 kronn

Can you provide a minimal, most basic YAML file that shows the issue?

DannyBen avatar May 16 '24 16:05 DannyBen

Sure. The above example can be reproduced by this:

rspec:
- --tag

rspec*--tag:
- sphinx:true
- type:controller
- type:model

My real setup fetches the possible tags from a script, but the problem shows both with hard-coded values and with the dynamic ones from the script.

kronn avatar May 16 '24 18:05 kronn

With the default behavior of bash completions - I am unable to make it work. It indeed gets stuck after the colon. I do not know how to solve this.

However, I am not using the default behavior of the bash completion, and it works beautifully as expected: cast1

To achieve this behavior, you need to have a ~/.inputrc file with this line in it:

TAB: menu-complete

This is my full ~/.inputrc` in case it helps:

# Change up/down arrows behavior
"\e[A": history-search-backward
"\e[B": history-search-forward

# Change tab behavior and add shift tab for old behavior
TAB: menu-complete
"\e[Z": complete

DannyBen avatar May 17 '24 04:05 DannyBen

That helps and works. Somehow, I do not like that behaviour, but since I am now in "personal preference"-land, my question is obviously answered.

Maybe I'll dive into bash-completion to search a solution that does work with complete as well while waiting for a spec or so. If I find something, I let you know.

For now, I am completely happy :-) Thanks for your help and completely.

kronn avatar May 17 '24 07:05 kronn

but since I am now in "personal preference"-land

Actually, since I am using the non-default behavior, I agree that this is still an interesting issue as to why it does not work as expected in the default mode. Colons are treated differently for some reason.

DannyBen avatar May 17 '24 08:05 DannyBen

I suspect some interplay with $IFS or $COMP_WORDBREAK or something similar. Most likely the latter. However, I have not yet played with setting this variable in the generated script or dug deeper into it. I know that changing this globally may break other completions (see https://github.com/npm/npm/commit/d7271b8226712479cdd339bf85faf7e394923e0d).

Since escaping the colon works and produces the expected result, it might also be a "simple" escaping/quoting issue.

kronn avatar May 17 '24 12:05 kronn

$COMP_WORDBREAK

This is it.

By removing colon from this variable, your use case works as expected: cast

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

  # Add this
  COMP_WORDBREAKS=${COMP_WORDBREAKS//:/}

  case "$compline" in
    *'--tag')
      while read -r; do COMPREPLY+=( "$REPLY" ); done < <( compgen -W "$(_rspec_completions_filter "sphinx:true type:controller type:model")" -- "$cur" )
      ;;

I do not want to add it to all of completely's output - but if there is a way to provide an elegant way to use a different template, or inject custom code to the generated script - I am happy to consider it.

DannyBen avatar May 17 '24 12:05 DannyBen

I see three options:

  1. add a dedicated setting to the YAML for configuring this This conflicts with the current assumptions the YAML suggests. Currently, everything in the YAML going into the completion-script as is. In order to not go against the implied assumptions, a key like _completely_options might work (with maybe in this case a hash named remove_from_wordbreak having a list of characters to remove). This might be handy in this case, but it could open the config for other extensions that are not intended. The beauty of the current state is that it is very easy to understand, which is a very welcome contrast to creating a completion-script manually.

The other two options are more automatic. The generator could check, if the completions contain a character from $COMP_WORDBREAK and

  1. remove it This might be removing too much if there is a typo and then breaking the completion for reasons that are hard to understand.

  2. provide a helpful warning how to fix this manually in the generated script This is the least invasive change I can think of which still somewhat resolves this.

Of course, the obvious fourth and fifth options are

  1. add the result of 3 to the README
  2. ignore this

but both are somewhat unsatisfying.

Which of these options would you like to see in completely (and maybe accept as a PR)?

kronn avatar May 19 '24 20:05 kronn

Well - thanks for outlining the options, but I am not crazy about any of them.

Keep in mind that there is also the consideration of bashly that uses this library, and I want it to continue operating properly, without the need to add a "completely options" section to the bashly configuration.

The option that seems simplest is 2 - just remove one or more characters from the COMP_WORDBREAKS variable for everyone, without any configuration.

The default character list is this: " \t\n\"'><=;|&(:"

In our context - where we know that each word specified in the YAML is a completion word - I wonder if it would do any harm in removing these:

  1. : colon - which exists here since it is a path separator
  2. ; semicolon - which exists here since it is a command separator
  3. = equal sign - which exists here since it is used in var=value

I am proposing this:

local original_comp_wordbreaks="$COMP_WORDBREAKS"
COMP_WORDBREAKS="${COMP_WORDBREAKS//[:;=]/}"

# <= the completions case statement goes here

COMP_WORDBREAKS="$original_comp_wordbreaks"

which I am testing with a completely.yaml like this:

asd:
- sphinx:true
- type:controller
- type:model
- completion=test
- completion;test
- <file>
- <directory>
- <command>

Thoughts?

DannyBen avatar May 20 '24 04:05 DannyBen

Being "crazy" about any of them would be a wild overstatement. :joy: Each one of them has some aspects that I do not like. Option 2 is the one that treats this question as a bug that should be fixed in completelys generated completion-script. I see it more of an edge-case that I also could "fix" by adapting the generated script, but an automatically (for me) working script is of course much more convinient. :smile:

Given the locality of the variable AND the reset to its previous value, tweaking it seems fine to me. Removing the = and : make sense to me.

I do have a bad feeling about removing the ; as this would - to my understanding - terminate the command if left unquoted. Therefore, it seems like an unusual choice for a character being included in a string that can be foreseen and hence completed. Or am I missing something here?

kronn avatar May 20 '24 11:05 kronn

about removing the ;

I don't think keeping it will cause issues, but in general I agree. I will remove it from.... ahm... the removal.

DannyBen avatar May 20 '24 11:05 DannyBen

I am closing this, as I do not see a way around it. If new information comes to light we can reopen.

DannyBen avatar Jul 06 '24 06:07 DannyBen

On Fri, Jul 05, 2024 at 11:47:42PM -0700, Danny Ben Shitrit wrote:

I am closing this, as I do not see a way around it. If new information comes to light we can reopen.

Fair enough. I think there might be a way, but I expect it to be a little more work. As I clearly do not have the time/energy for this right now, closing it seems like the appropriate move.

kronn avatar Jul 07 '24 20:07 kronn