Not adding space after dot at completion time

[binshengliu]

Hi, I'm implementing fish completion for hydra. It is a framework to manage configuration key/values in a hierarchical style. For example to specify a configuration from command line is like ./script.py db.name=sqlite. It also has support for dynamic completion so I'm integrating it with fish using dynamic completion: complete -c test.py -x -a '(test.py -sc query=fish --_completion (commandline -cp))', following an earlier discussion.

The problem is fish automatically adds a space after candidates ending with a dot .. Completing ./script [TAB] results in ./script db. . For further completion the user needs to delete the space and press TAB again. But for = it works as expected. (asciinema recording: https://asciinema.org/a/322303)

I'd like to suggest that space is not added after . like the behaviour for =, so the dot can also be used as a separator in such scenarios.

Environment:

$ fish --version
fish, version 3.0.2
$ echo $version
3.0.2
$ uname -a
Linux debian 4.19.0-8-amd64 #1 SMP Debian 4.19.98-1 (2020-01-26) x86_64 GNU/Linux
$ echo $TERM
xterm-256color

[faho]

Fish will insert a space whenever a candidate has been unambiguously accepted.

If you want to then continue the token, you'll have to make longer candidates.

E.g. if a command foo has the completion candidate bar., fish will insert a space after that. But if it also has a candidate bar.baz and bar.bonk, it won't insert a space after the bar. because there are still possible completions.

There is indeed a teensy special case for =, but that's because = is used as a separator in lots of things. . isn't, so making it special doesn't, on the whole, help.

[binshengliu]

Thanks for the reply. I understand your consideration on the whole. Is it possible this can be customized so apps like hydra have a chance to tailor to what they need?

[krobelus]

The simplest solution is to generate all possible tokens, not only the thing until the next .. Fish will automatically filter them for you based on the current token.

Say you have the options:

foo:
  bar:
    baz: 1
  qux: 2

You would generate two completions:

foo.bar.baz=
foo.qux=

[omry]

Hi,
I am the author of Hydra.

The flattened tree (actually trees) can be massive and it makes no sense to return the full list.
what other solutions are there?

I remember hitting a related problem with bash where I had to change the word break characters, removing / as a word break.

Does fish support controlling the word break characters as well?

[krobelus]

Hmm yeah if the number of leaves in the forest is in the 5-digits, then it's probably not the best idea.

There is no proper support for changing the word separators.
However, to solve the problem of "not adding a space after a completion", fish has since forever been using a hack which does exactly this if the completion ends in /, =, @ or :.

fish-shell/src/complete.cpp

Line 200 in 18efd7d

if (len > 0 && (std::wcschr(L"/=@:", comp.at(len - 1)) != nullptr))

Extending this hard-coded list by . would solve this issue. Also adding , would improve upon #6833. I think we could get away with this. Are there any completions that end in non-separating. or ,?

[omry]

Hi @krobelus.

This problem caught me off guard: I have a pretty solid comprehensive test suite for the completion and it misses it. (The testing I have is another reason I would like a behavior change: I have a single test suite I am running for bash and now for fish, and in the future for other shells as completion plugins are implemented.

I will want to add some more tests that are testing the case for two consecutive completions in various cases to be sure there isn't anything else. I suspect adding '.' to that list would address my needs though.

I also need / and = for a different kind of completions, it's good to see those are covered, and I am actually thinking about using @ as a part of the syntax in the future so great to see it here as well.

Are there any thoughts about allowing the control of those work break characters through an environment variable? (FISH_WORD_BREAK?)
I can imagine any decision here might be inappropriate for some uses cases and while it's good to reasonable defaults it's also good to allow for changes if someone really needs it.

[faho]

Are there any thoughts about allowing the control of those work break characters through an environment variable? (FISH_WORD_BREAK?)

Two thoughts: That would be quite unfishy, and a global option would be entirely unsuited here.

As you've said, all your needs are covered by adding the two additional characters, and fiddling with low-level crap like this isn't something we want to subject our users to. Adding a knob here would make things worse, not better.

If it does become apparent that more is needed (and it doesn't seem like it currently) we should add a flag to complete instead of a global option, so that the completion can decide that it needs to stop at that point.

[krobelus]

This problem caught me off guard: I have a pretty solid comprehensive test suite for the completion and it misses it.

I see, you are even simulating an interactive process with expect. Fish is in the process of switching to pexpect, is there a particular reason for sticking with the former?

Are there any thoughts about allowing the control of those work break characters through an environment variable? (FISH_WORD_BREAK?)

It's a very specific hack only affecting completions, not any other word splitting so this has potential for confusion. Once it doesn't cut it anymore we should instead add a new flag to complete as @faho said.

[omry]

This problem caught me off guard: I have a pretty solid comprehensive test suite for the completion and it misses it.

I see, you are even simulating an interactive process with expect. Fish is in the process of switching to pexpect, is there a particular reason for sticking with the former?

I remember I looked at pexpect briefly and decided against it, can't remember why. I think it was not exactly following the behavior expected (Pun!) by Expect, can't remember what was the issue though.
Now that you mention it, I think I was really happy to see it and really disappointed to find out that it didn't cut it.

Are there any thoughts about allowing the control of those work break characters through an environment variable? (FISH_WORD_BREAK?)

It's a very specific hack only affecting completions, not any other word splitting so this has potential for confusion. Once it doesn't cut it anymore we should instead add a new flag to complete as @faho said.

Gotcha.
an additional flag to complete would be great.

[omry]

Thanks @krobelus!
I am a bit uneasy to see such a change without any testing done to ensure that it remains fixed :)
Do you have an estimated time for when this will be available in downstream repos?

@binshengliu, can you build fish from master and check your completion plugin with the updated fish?

[binshengliu]

Thanks for the change @krobelus.

I experimented with this change before and it works @omry.