feat(completion): add fish shell completion - #15515
Conversation
Signed-off-by: Chizaram Chibueze <chizy7@gmail.com>
| let completion_script ~fun_name = | ||
| sprintf | ||
| {|function %s | ||
| set -l tokens (commandline -opc) |
There was a problem hiding this comment.
According to the fish docs -o is deprecated https://fishshell.com/docs/current/cmds/commandline.html. In the changelog they suggest to use -x instead https://fishshell.com/docs/current/relnotes.html#id42
There was a problem hiding this comment.
Ok, I can update it to try -x first and fall back to -o on older fish:
set -l tokens (commandline -xpc 2>/dev/null)
or set tokens (commandline -opc)Should I keep this fallback, or drop it and require fish >= 4.0?
There was a problem hiding this comment.
I don't use fish personally, so I'll leave that to fish users @ElectreAAS @Leonidas-from-XIV to decide. Since fish isn't the main shell of any distro, I don't think it should be any issue to require a newer version. If people have gone out of their way to setup fish, they are probably able to get their hands on a newer version anyway.
There was a problem hiding this comment.
In general I would pick whatever it is that you think would be easier to maintain going forward. If the fallback works then so be it.
There was a problem hiding this comment.
I'll keep the fallback then. it's one extra line. I have pushed it.
There was a problem hiding this comment.
I always just use whatever fish comes with the distribution and would be too lazy to specifically install a newer fish. That said, fish 4.0+ is in a lot of distributions including Debian stable, so I think requesting 4.0+ is ok now and even more ok in the future.
Signed-off-by: Chizaram Chibueze <chizy7@gmail.com>
ElectreAAS
left a comment
There was a problem hiding this comment.
In the current state of things it works, and everything looks okay at a cursory glance
I'm not a huge fan of adding code that nobody in the team understands, but hopefully we can upstream all of this to cmdliner soon, reducing the maintenance burden on our end
Thanks a lot :)
|
Thank you very much @chizy7! |
Description
adds
fishto the shells supported bydune completion, folowing the work in #14779 which added bash, zsh, and powershell but left fish out.Since cmdliner doesnt provide a generic fish completion script (
dbuenzli/cmdliner#213), this PR adds one on the Dune side as a newFishmodule inbin/completion.ml. It can't live undervendor/cmdlinerbecause that directory is overwritten byvendor/update-cmdliner.sh. The implementation is mostly generic apart from the command name, so it could potentially be upstreamed to cmdliner later.What's Included
--root=.restartandmessage.What's Not Included
dune build <TAB>→@test,@install, etc.), matching the existing shell implementations.messagedirectives. Fish has no safe way to show completion messages without interfering with the pager, so they are consumed buut not displayed.Notes for Reviewers
commandline -opcfor compatibility with both fish 3.x and 4.x.To try it:
Related Issue and Motivation
Fish users currently don't get shell completions from
dune completionbecause cmdliner has no fish support. This PR adds a Dune-maintained implementation following the existing completion support, with the possibility of upstreaming it to cmdliner in the future.Fixes #15334.
Checklist