-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow completers to disable unwanted shell behaviors #414
base: master
Are you sure you want to change the base?
Conversation
This changes the protocol from Haskell to the shell integrations. It allows more room for new types of information to be passed to the shells; even shell-specific things that do not affect other shell because the integrations will just ignore the new I propose to add to the wiki page that explains the integration: The list of completions is written to stdout in a format that looks like
Each completion is preceded by a New "keywords" can be added for new hints that affect one or more of the shell implementations. Those can potentially make use of the rest of the line to carry extra information about the hint. |
Cheers. I haven't had a hard look at this yet, but I'm all for richer and better completions. I would really like our output to be backwards compatible though. People often don't change their completion scripts for years, while their optparse equiped binaries might upgrade to a new version at any time. If we're going to develop a richer interface, it should be behind a separate command or flag. There's already one of these for "richness". |
I'm doing a bit of background research over here. I've seen a few small proposals for standarisation of completion scripts, so I'm looking at whether this functionality is supported in those. I would, in general, like to encourage more tools in more languages into doing what optparse-applicative does. Writing custom completion scripts in bash is a ridiculous exercise IMO. |
I've added a version option for compatibility and I've changed the CompletionItemOption monoid. I haven't tested the updated shell integration scripts yet. Did you find a useful set of standard completion scripts? |
It took me a while to find this. It's a proposal for essentially a protocol like optparse's. https://github.com/oilshell/oil/wiki/Shellac-Protocol-Proposal I'll let you have a read and I will go into it further later this week. We can then discuss if it's up to task. |
The shellac protocol looks ok at first, but I've found some issues.
I hope the first one is the only real issue, so I'll ask about it. |
Yeah I noticed that as well. |
, " value_mode=true" | ||
, " ;;" | ||
, " %addspace)" | ||
, " compopt +o nospace" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this name flipped? addspace means we use nospace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to avoid negations in names, so in the protocol it's addspace
which is the inverse of bash's nospace
. The inversion is achieved by +o
which is the inverse of -o
.
So I've simplified the protocol semantics slightly, at the cost of some implementation complexity in the bash integration.
, " compopt +o nospace" | ||
, " ;;" | ||
, " %files)" | ||
, " compopt -o filenames" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what your thoughts are about delegating back to the completion script for file names instead of doing the bash dance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems like a good idea for another PR. It makes #408 obsolete and removes the dependency on the ancient bash that comes with macOS.
Overall I think this is looking very reasonable. It's not too complex a protocol. We'll see if anyone gets back to us regarding shellac. |
adding a completionItem makes sense IMo, I did it too to ease some integration with haskeline https://github.com/teto/optparse-applicative/blob/ad5d43ad8f4cbca13ca4d0658926e81a2d1e7143/src/Options/Applicative/Types.hs#L332 |
Is this blocked on rebase or is there something else I need to change? |
@roberth I'd certainly appreciate it, I've just been totally baffled as to why some completion was getting a trailing forward slash in a tool I built and only just stumbled on this. |
96473f0
to
2e4737f
Compare
I've started a draf spec for the protocol, calling it ACES: Auto-Completion for Executables and Shells. It's close to what this PR implements, but uses |
2e4737f
to
15ce2aa
Compare
This pr adds
mkCompleterWithOptions :: (String -> IO [CompletionItem]) -> Completer
, which allows the completer to disable unwanted shell behaviors likeThe latter is useful for hierarchical path completions that aren't in the filesystem. Shell come with hacks to make the filesystem case work well, but you need to disable those if you need to implement your own path-like completions.
zsh
andbash
have facilities to override this behavior dynamically.zsh
does so per match, whereasbash
has parameters we can set per invocation, which seems to work equally well.fish
doesn't have any of this and decides heuristically based on the last character.A
CompletionItem
is essentially a tuple of aString
andCompletionItemOptions
, whereThe smart constructor
mkCompleter
remains unchanged, which is great for backwards compatibility. TheTypes
module does expose theCompleter(Completer)
constructor, but I suppose most packages use theOptions.Applicative
module.