CliFx icon indicating copy to clipboard operation
CliFx copied to clipboard

Autocompletion

Open Tyrrrz opened this issue 4 years ago • 22 comments

Investigate what's possible

Tyrrrz avatar Aug 14 '19 21:08 Tyrrrz

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.

domn1995 avatar Apr 21 '20 21:04 domn1995

Yeah I seem to remember that Windows had a similar setup where the user had to configure something once.

Tyrrrz avatar Apr 21 '20 21:04 Tyrrrz

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 .EnableAutoCompletion() on CliApplicationBuilder.

Tyrrrz avatar May 11 '20 18:05 Tyrrrz

Looking for a little more info on the EnableAutoCompletion() API and throwing out some ideas.

  • What shells do we generate for?
    • All supported shells?
    • Only for those installed on the user's system?
    • Or do we let the CliFx consumer configure this behavior? Maybe through a AutoCompletionOptions object that CliFx users can configure and extend?
  • Do we want to support CliFX consumers extending the autocompletion functionality on their own projects or must it happen within CliFx?
  • How do we know whether we need to regenerate the autocompletion scripts or not?
    • If we regenerate every time, are we just overwriting the previous ones?
  • What about providing a built-in command to enable autocompletion like myapp install-autocompletion <shell> where <shell> are the supported shell types? I like this idea because it is explicit. By default it could install for the current shell.

domn1995 avatar May 12 '20 18:05 domn1995

What shells do we generate for?

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?

Do we want to support CliFX consumers extending the autocompletion functionality on their own projects or must it happen within CliFx?

I don't think it makes sense to extend it on consumer's end.

How do we know whether we need to regenerate the autocompletion scripts or not?

That might be a problem. An alternative approach would be to add a new directive [enable-autocompletion] that would do it on-demand. Maybe let the user know about it in --help.

What about providing a built-in command to enable autocompletion like myapp install-autocompletion where are the supported shell types? I like this idea because it is explicit.

I would prefer a directive for that because that's the interface for cross-cutting concerns.

Tyrrrz avatar May 12 '20 18:05 Tyrrrz

What shells should we support? I guess PowerShell, bash, anything else?

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?

I don't think it makes sense to extend it on consumer's end.

Agreed. They should extend it in CliFx.

I would prefer a directive for that because that's the interface for cross-cutting concerns.

Oh yeah, agreed! Totally forgot about directive support. Another reason this project is awesome!

domn1995 avatar May 12 '20 18:05 domn1995

😊

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?

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.

Tyrrrz avatar May 12 '20 18:05 Tyrrrz

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.

domn1995 avatar May 12 '20 19:05 domn1995

Here's how autocomplete installation works for dotnet: https://docs.microsoft.com/en-us/dotnet/core/tools/enable-tab-autocomplete

And here's the code from the generating the suggestions for the dotnet complete command: https://github.com/dotnet/sdk/tree/master/src/Cli/dotnet/commands/dotnet-complete

I think emulating this is probably the way to go, and it actually seems relatively simple.

domn1995 avatar May 15 '20 05:05 domn1995

I see, so it actually will need 2 commands, one to setup autocomplete and one to query it.

Tyrrrz avatar May 15 '20 07:05 Tyrrrz

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'm unsure if dotnet-suggest uses dotnet-complete underneath.

LordLyng avatar Jul 28 '20 08:07 LordLyng

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 dotnet-suggest has been downloaded so far.

Ideally, it should be fully automatic or at least configurable directly from within the same CLI tool.

Tyrrrz avatar Jul 28 '20 10:07 Tyrrrz

I've used dotnet suggest-powered completion before and the lag between hitting tab and getting a completion can be quite annoying.

jcotton42 avatar Dec 15 '20 23:12 jcotton42

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 dotnet CLI, but can be adapted for general case).

Tyrrrz avatar Feb 12 '21 17:02 Tyrrrz

Spiked this today. Interesting.

I added a [autocomplete] directive which works as follows:

> clifx.demo [autocomplete] b
book add
book
book list
book remove

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.

mauricel avatar Mar 02 '21 16:03 mauricel

Perhaps [suggest] is a better name?

adambajguz avatar Mar 02 '21 17:03 adambajguz

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 eg. cli.exe [suggest] --var clifx-suggest-{guid} --cursor {cursor-position}

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 My spike has a great deal of extra code. A better design might be to have CommandInput extract raw tokens as part of its parsing process, then pass these tokens to a service to provide sensible suggestions. I wanted to seek input before modifying existing classes. See https://github.com/mauricel/CliFx/tree/%234-spike-unify-parser-code

mauricel avatar Mar 23 '21 17:03 mauricel

@mauricel

Design suggestion 1: Support passing of auto-complete text by environment variable

I like the idea of passing parameters via environment variables. It's cleaner and probably easier for us.

Design suggestion 2: Fallback support passing of auto-complete text by command line

Sounds good.

Design suggestion 3: Enhance CommandInput to return raw tokens

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?

Tyrrrz avatar Mar 23 '21 23:03 Tyrrrz

Some more inspiration: https://github.com/clap-rs/clap/blob/master/clap_generate/src/generators/shells/powershell.rs

Tyrrrz avatar Mar 23 '21 23:03 Tyrrrz

Hmm, can you explain how this will benefit us?

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.

Can we not pass parsed inputs to suggestion service instead?

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.

Also, if we need this, can we have each input encapsulate its own token(s), so it's not kept in a separate object?

Yeah, that makes sense.

Thanks for the design review!

mauricel avatar Mar 24 '21 14:03 mauricel

... Let me explore here a bit more

Seems like we won't need it for now. Thanks for the sense check.

mauricel avatar Mar 24 '21 16:03 mauricel

No problem 🙂

Tyrrrz avatar Mar 24 '21 19:03 Tyrrrz