-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Autocompletion #13
Comments
This looks like a promising tutorial on how to do it for bash: https://iridakos.com/programming/2018/03/01/bash-programmable-completion-tutorial The tricky part is we would need the completion script to be copied to a user's bash directory at the time of installation of the CLI tool. I don't think that functionality belongs in CliFx, but we can definitely provide a method of generating the completion script from the command schema and leave it up to CliFx developers to bundle it with their app or not. |
Yeah I seem to remember that Windows had a similar setup where the user had to configure something once. |
So it seems that we will indeed need to configure it once per shell. Suggested design: When the CliFx application executes for the first time, run appropriate scripts for each supported shell (PowerShell, Bash, ...?) to register auto-completions. Since it's quite a heavy operation and potentially damaging, make it disabled by default, gated by |
Looking for a little more info on the
|
Good question. Ideally for the currently running shell if it's reasonably practical to get that information. Otherwise I guess just try to enable for all supported shells and ignore errors in case they are not available. What shells should we support? I guess PowerShell, bash, anything else?
I don't think it makes sense to extend it on consumer's end.
That might be a problem. An alternative approach would be to add a new directive
I would prefer a directive for that because that's the interface for cross-cutting concerns. |
I think supporting these two for initial feature release is perfect. Maybe Windows Command Prompt as well? We should also design it so extending it within CliFx to add support for other shells is pretty painless. Is this a design goal that you agree with?
Agreed. They should extend it in CliFx.
Oh yeah, agreed! Totally forgot about directive support. Another reason this project is awesome! |
😊
If it's possible, yes. But I think it's more important to get coverage for the most common shells than to make it extendable. Let's see how it goes and adapt. |
Agreed. I'm currently working on #17 and then I can start prototyping this. |
Here's how autocomplete installation works for And here's the code from the generating the suggestions for the I think emulating this is probably the way to go, and it actually seems relatively simple. |
I see, so it actually will need 2 commands, one to setup autocomplete and one to query it. |
dotnet utilizes dotnet-suggest (https://github.com/dotnet/command-line-api/wiki/dotnet-suggest) in their experiemental System.CommandLine (https://github.com/dotnet/command-line-api) library. |
I think the user experience where you have to install a global tool and then also configure it for your terminal manually is completely horrible. It's also probably evident by how much Ideally, it should be fully automatic or at least configurable directly from within the same CLI tool. |
I've used |
Relevant link: https://docs.microsoft.com/en-us/dotnet/core/tools/enable-tab-autocomplete Examples on how to enable tab completion in various terminal (for |
Spiked this today. Interesting. I added a [autocomplete] directive which works as follows:
I then registered the ArgumentCompleter with powershell to call the application using the autocomplete directive. So far, I've only toyed with commands so far, might deep dive a bit later. |
Perhaps |
I'd like to make the following suggestions to progress this ticket forward. Design suggestion 1: Support passing of auto-complete text by environment variable Powershell and Bash supply a text variable containing the unmodified autocompletion text (string), and a cursor position variable (int). Passing the text variable as a command line argument in powershell is unreliable. Powershell has breaking weirdness in the way variables are split, then passed to the CLI. I found I got better results by stuffing the text variable into an environment variable, and passing the environment variable name to in the suggest directive. The other issue I found was that the cursor position is not usable unless we had access to the original text. I recall having issues because Environment.CommandLine doesn't provide access to the original text -- it may be escaped. Regarding safety of environment variables: Powershell environment variables are process scoped, and can be deleted after processing. I think those should be okay. I don't think Bash environment variables support such scoping, however I think this approach should be okay. Otherwise one can fall back to just passing auto-complete text by command line. Design suggestion 2: Fallback support passing of auto-complete text by command line eg. cli.exe [suggest] {autocomplete-text} The .net documentation above describes a use case (zsh) where only the text is available. I think we should also provide this as a fallback. Design suggestion 3: Enhance CommandInput to return raw tokens |
I like the idea of passing parameters via environment variables. It's cleaner and probably easier for us.
Sounds good.
Hmm, can you explain how this will benefit us? Can we not pass parsed inputs to suggestion service instead? Also, if we need this, can we have each input encapsulate its own token(s), so it's not kept in a separate object? |
The code in CommandInput.Parse() is currently is responsible for splitting the argument array into directive, command, parameter and option components. My first naive approach simply re-implemented that behaviour (somewhat), but I didn't like how there were two (slightly different) implementations. I found bugs because I'd misunderstood how aliases worked. The benefit I was aiming for was to keep the responsibility of splitting arguments up into command, parameter, components in one place.
Let me explore here a bit more. Ideally, that's what I'd like to do. Yesterday, I wasn't sure how we can use CommandInput for partial string matching of Commands. I didn't like pulling command information out of the CommandInput.Parameters fields too much so I thought passing the parser's "state" to the service would be better. Upon second thought, I see an issue with having two models of the parser output that don't match.
Yeah, that makes sense. Thanks for the design review! |
Seems like we won't need it for now. Thanks for the sense check. |
No problem 🙂 |
Investigate what's possible
The text was updated successfully, but these errors were encountered: