Raku / App-Rakubrew

Raku environment manager
https://rakubrew.org/
Other
26 stars 13 forks source link

Fix completion for Fish #22

Closed vrurg closed 4 years ago

vrurg commented 4 years ago

The completion is currently plain broken. This PR currently works for me.

Also fixed build-macos.sh which doesn't work if PERL5LIB shell variable is set due to cpanm installing elsewhere but not into the local perl copy. Looks like the same fix would be required for other build scripts but I better don't touch them without trying myself.

And likely a fix for a problem with Zsh.pm which incorrectly expects $index as its first argument.

vrurg commented 4 years ago

@JJ Test for what? I don't see other tests in the project, no idea what would be the right approach for the new ones.

JJ commented 4 years ago

@JJ Test for what? I don't see other tests in the project, no idea what would be the right approach for the new ones.

Only when it's accepted...

vrurg commented 4 years ago

The substring (seed) matching for completion is implemented but it has certain issues with fish and zsh.

For fish things work almost as expected except that if there're candidates to which the seed is their prefix then it automatically chooses the shortest candidate without offering any choice to the user. I.e., for ver with given candidates version, versions, and rakubrew-version fish will auto-substitute version in the command line ignoring two other. With second tab it allows to chose among version or versions, still ignoring rakubrew-version. I didn't find a workaround for this behavior. But rakubrew works as expected and handles everything correctly anyway.

BTW, I also had to change the suggested completion line for fish because commandline -poc only returns full tokens up to the cursor position and skips over the one where the cursor is located. Additional commandline -ct following the -poc gives the missing seed for completion.

For zsh situation seems to be less pleasant because it seemingly only capable of handling candidates where the seed is the prefix. I.e. in rakubrew build moar 08<tab> it offers no menu of variants. I haven't found a solution for this.

UPD compctl -U fixes the problem in zsh.

Can't test other shells for now.

vrurg commented 4 years ago

BTW, I also had to change the suggested completion line for fish because commandline -poc only returns full tokens up to the cursor position and skips over the one where the cursor is located. Additional commandline -ct following the -poc gives the missing seed for completion.

From local tests with fish 3.0.2 (adding say "XX${command}YY"; to sub completions) this is not the case. commandline -poc does return the last partial token. Unless there was a behavioural change in fish between the version I use and yours, something else must be off.

Mine is v3.1.2. The exact complete which works for me now is:

complete --no-files -c rakubrew -a '(command /Users/vrurg/.rakubrew/bin/rakubrew internal_shell_hook Fish completions (commandline -poc) (commandline -ct) | string split " ")' -n _rakubrew_is_not_register

There is nothing in the release notes which would justify this change in behavior. Likely to be a regression because here is what I get as arguments for rakubrew bui<tab>: <ARGS:{internal_shell_hook}{Fish}{completions}{rakubrew}{bui}>.

BTW, I don't keep complete in config.fish. Instead I have individual completion files in ~/.config/fish/completions. But this is unlikely to affect the issue.

Another thing I realized that might be related: Because the index is currently not passed, it's necessary to increase the calculated index if the command ends with a space (to be able to complete a new word for which no letter was yet typed). To retrieve that last space one needs to pass the command as a single argument. (commandline -pc) should do that. That same logic is already part of the Tcsh implementation, which has the same issue. You can copy from there.

And, again, I don't have this poblem. 'rakubrew build <tab>' results in all jvm moar moar-blead menu. Could be due to the additional (commandline -ct) which results in extra empty argument: <ARGS:{internal_shell_hook}{Fish}{completions}{rakubrew}{build}{}>.

Unrelated, but did you know that the rakubrew source tree is a valid Perl module and can be installed using cpanm .? That's how I test locally.

It would result in more handwork to me. If I install it locally with cpanm then I'd need to revert the installation later when done with this fix. It's easier to run the script and have it installed into the standard location. :)

vrurg commented 4 years ago

Update to the situation about fish. CentOS, fish v2.3.1, no custom settings whatsoever. My completion string works in full as expected, including space after the last keyword. Looks like if there is a regression then it was in v3.0.2 or v3.0 as such.

patrickbkr commented 4 years ago

I looked into this a bit deeper. My diagnosis was faulty. It behaves on my machine exactly as you describe. I was looking at the result of (commandline -pc) which indeed does return the last partial token. But (commandline -poc) behaves just as you observed and omits the last token. When thinking about it it does make some sense.

So everything looks good! Merging! Thanks for your work and persistence with this!