Tab completion in zsh broken in 0.9.5?

[VorpalBlade]

It appears at some point tab completion broke for my project https://github.com/VorpalBlade/chezmoi_modify_manager using bpaf, it doesn't work with version 0.9.5 any more at least.

$ cat /usr/share/zsh/site-functions/_chezmoi_modify_manager
#compdef chezmoi_modify_manager
source <( "${words[1]}" --bpaf-complete-rev=7 "${words[@]:1}" )

It can no longer complete the flags, only files.

Looking at the output manually shows this too:

$ chezmoi_modify_manager --bpaf-complete-rev=7 --a
compadd -- --a

which is odd since:

$ chezmoi_modify_manager --help                     
Add-on for chezmoi to handle mixed settings and state

Usage: chezmoi_modify_manager (FILE | -a [-t=STYLE] [FILE]... | -s [-t=STYLE] [FILE]... | --help-syntax
| --help-transforms | --doctor | -u)

Available options:
    -a, --add              Add a file to be tracked by chezmoi_mm
    -t, --style=STYLE      How to call the modify manager in the generated file [path (default), src]
    -s, --smart-add        Smartly add a file to be tracked by either chezmoi or chezmoi_mm
        --help-syntax      Print help about about the config file syntax
        --help-transforms  Print help about supported transforms
        --doctor           Perform environment sanity check
    -u, --upgrade          Perform self update
    -h, --help             Prints help information
    -V, --version          Prints version information

Here is the source code for the parser: https://github.com/VorpalBlade/chezmoi_modify_manager/blob/e7d117943b5b0522eadcb6ab00fa8aa524926572/src/arguments.rs

It has definitely worked before, but the command is primarily called indirectly by scripts or other commands, so I don't know exactly when it broke.

EDIT: I wrote zsh in the title since I don't use other shells and don't have tab completion for this command set up in them.

[pacak]

Will take a look in a few hours/tomorrow.

[pacak]

Problem is related to having "value" items (complete_shell(ShellComp::File{mask: None})) completing at the same time as named items (--add, etc). Past me decided that "value" items take priority here but didn't leave any comments explaining why. This change was made back then when I had to test all the completions manually, now that I have comptester it should be easier to make sure that all the invariants hold.

As a quick fix if you remove complete_shell annotations most of the completions should start working again, other than that - I'll try to make a proper fix it in the next version.

[VorpalBlade]

As a quick fix if you remove complete_shell annotations most of the completions should start working again, other than that - I'll try to make a proper fix it in the next version.

I tried that, and it certainly improved. However chezmoi_modify_manager --<tab> only completes to exactly --. It should really list --add, --smart-add, --doctor, --upgrade, --help, --version, --help-syntax and --help-transforms too. Perhaps this is a separate bug?

As soon as I add another letter (e.g. chezmoi_modify_manager --h<tab>) it works as expected.

EDIT: I assume bpaf adds builtin support for -- (i.e. to treat the remaining arguments as plain positional arguments), it is certainly not something I have added myself as far as I know.

[pacak]

Hmm... Probably still related to positional items. Making slow progress on the next version so hopefully soon :)

[pacak]

Rubberducking/note to self.

Let's consider a minimal possible parser that picks between adding a FILE (a positional parser with attached shell completion) and a flag -a.

The way completion works for alternative branches is by trying to run both branches and picking completion results from both (if both branches fail the same way or succeed) or one that succeeds (or tries harder (*)), so to get completion both need to produce the same result.

To be able to tell the difference between user trying to complete foo --bar and foo --bar shells pass an empty string as one of the arguments, bpaf follows the same convention.

With flag -a situation is simple - "" is no a short name a so it fails. With FILE - "" is a valid positional, so parser succeeds, then combination of them retains information from that side only as a result completion looks broken.

I can tell the difference between something passed by user and something inserted by the shell for completion purposes and make positional parser to fail on "" this way, but this would break a second piece of logic that allows to generate dynamic completion in the first place when there's no input. Tricky.

I can't even mark it as failing in a second layer since this counts as failing while trying harder.

Possible options I see so far are:

1. Make or_else somehow aware of this kind of combination
2. Pick results from both branches more aggressively and deal with possible conflicts somewhere else
3. Change how completion for placeholders work

(2) seem like the easiest way forward for now. Either way I need to add more internal documentation about assumptions made for completion - trying to remember how it works is hard.

[VorpalBlade]

I don't know the internals of bpaf, but is there no additional metadata that could be useful that the shell provides? I have hand written zsh completion files in the past, and it provides a host of useful environment variables, such as cursor position etc

(I don't know about the other shells, and what they provide likely differs, so maybe not very useful to you.)

[pacak]

All the shells provide more or less the same info and I don't think any additional info from the shell will help, it's all about the internal representation. Anyway, after an hour of cleaning up the front yard I came up with some more ideas to try, a variation on (2), but deciding what completions to keep depending on what was consumed and how it might conflict. Now I'm back to experimenting :)

[pacak]

Got something working. Can you check if branch complete works for you?

[VorpalBlade]

Got something working. Can you check if branch complete works for you?

Seems to work well in zsh at least.(NOPE, see next comment) But with bash I notice it only completes flags (and only some of them) now (before it only completed files).

As for the other shelles (fish etc) I don't have those installed, so can't help there

[VorpalBlade]

The behaviour with bash seems really broken in fact:

$ target/debug/chezmoi_modify_manager <tab>
--style=STYLE            -- How to call the modify manager in the generated file [path (default), src]
--smart-add              -- Smartly add a file to be tracked by either chezmoi or chezmoi_mm
--style=STYLE            -- How to call the modify manager in the generated file [path (default), src]
--help-syntax            -- Print help about about the config file syntax
--help-transforms        -- Print help about supported transforms
--doctor                 -- Perform environment sanity check
--upgrade                -- Perform self update
--no-confirm             -- Do not ask for confirmation before applying updates

$ target/debug/chezmoi_modify_manager -<tab>
# Changes it to:
$ target/debug/chezmoi_modify_manager --s
--smart-add              -- Smartly add a file to be tracked by either chezmoi or chezmoi_mm
--style=STYLE            -- How to call the modify manager in the generated file [path (default), src]

And in zsh it actually breaks as well:

What happened to --upgrade etc?

Some misc additional issues:

• Another weird one (in zsh) is that --h<tab> goes to --help- with a list of the two options. What happened to --help itself?
• Short flags don't seem completable at all.
• It doesn't seem to have context and understand "modes". I.e. --no-confirm is only valid with --upgrade, and you can't have both --upgrade and --add. Completion seems to think that any combination goes. It really should have some understanding of the enum-ness of chezmoi_modify_manager (FILE | -a [-t=STYLE] [FILE]... | -s [-t=STYLE] [FILE]... | --help-syntax | --help-transforms | --doctor | -u [--no-confirm])

[pacak]

Hmm... Looking...

[pacak]

Reproduced the problem you are having locally by copying your whole parser configuration in my tests :) Fixing.

[pacak]

Took some thinking time, pushed a new version of complete branch. Problem was related to how - and -- are being in between of named and positional arguments.

[VorpalBlade]

Hm, not quite yet: Here is a weird one in zsh:

$ target/debug/chezmoi_modify_manager -a <tab>
$ target/debug/chezmoi_modify_manager -a --style

That should have accepted filenames as well. --style (also known as -t) is not a required argument.

$ target/debug/chezmoi_modify_manager --he<tab>
$ target/debug/chezmoi_modify_manager --help-

Plain old --help also exist (as well as --help-syntax and --help-transforms)

$ target/debug/chezmoi_modify_manager -u <tab>
# List of files and all global flags, like --doctor, neither of which is correct, the only real one is --no-confirm which is a valid optional flag.
$ target/debug/chezmoi_modify_manager -u --doctor
Error: `--doctor` cannot be used at the same time as `-u`
# That is what I expect, but tab completion didn't understand

Seems tricky to get this right!

[pacak]

That should have accepted filenames as well. --style (also known as -t) is not a required argument.

Hmm... Instruction to zsh to accept filenames is generated though... I wonder. _files doesn't work together with compadd?.... Time to do some more research :)

[VorpalBlade]

That should have accepted filenames as well. --style (also known as -t) is not a required argument.

Hmm... Instruction to zsh to accept filenames is generated though... I wonder. _files doesn't work together with compadd?.... Time to do some more research :)

Ouch, I should also warn that my zsh setup is not exactly vanilla. So I hope I'm not leading you down a wild goose chase here. Please check that you can reproduce it first! I'm still using the (sadly no longer really maintained) zsh4humans. I should probably get around to using something else. Any year now. ;)