Skip to content
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

Autocomplete feature #48

Open
bilal-fazlani opened this issue Jul 13, 2018 · 21 comments
Open

Autocomplete feature #48

bilal-fazlani opened this issue Jul 13, 2018 · 21 comments

Comments

@bilal-fazlani
Copy link
Owner

Let's try n use this library to provide autocomplete for commands & subcommands first. Then if possible, we can incorporate autocomplete for option names too.

@bilal-fazlani bilal-fazlani self-assigned this Jul 13, 2018
@bilal-fazlani bilal-fazlani added this to To Do in CommandDotNet via automation Jul 13, 2018
@bilal-fazlani
Copy link
Owner Author

github url for auto-complete library : https://github.com/tonerdo/readline

@bilal-fazlani
Copy link
Owner Author

Change of plans:
It's not possible to use "Readline" library for this. That library is supposed to help you provide autocomplete once you are IN the program. for our purpose, we need to get help from bash.

Here's an example: https://github.com/iridakos/bash-completion-tutorial

@bilal-fazlani
Copy link
Owner Author

The feature is being developed here: https://github.com/bilal-fazlani/commanddotnet/tree/autocomplete-new

@drewburlingame
Copy link
Collaborator

I wonder if we can tap into Suggestions from System.CommandLine

https://github.com/dotnet/command-line-api/wiki/Features-overview
https://github.com/dotnet/command-line-api/wiki/dotnet-suggest

And dotnets autocomplete. Even if this only worked for dotnet to start, it would be a win

https://docs.microsoft.com/en-us/dotnet/core/tools/enable-tab-autocomplete

@drewburlingame
Copy link
Collaborator

Another nugget for thought. Bash completion supports the keywords file and directory so that files and directories are suggestions. It would be handy if we used those keywords with the argument type was FileInfo or DirectoryInfo.

@bilal-fazlani
Copy link
Owner Author

@drewburlingame

It would be handy if we used those keywords with the argument type was FileInfo or DirectoryInfo.
Nice thought. 👍

I had some vision about this feature when I was actively developing on this repo.

Static out of the box autocomplete

  • command & subcommand autocomplete
  • named argument autocomplete

Example:

  1. git checko and then a TAB would complete the command checkout

Data driven autocomplete

  • Developers should be able to write a method in C# which would be executed to fetch autocomplete results. Developers can provide a static hardcoded list of results or fetch results using HTTP calls to their servers.

Examples:

  1. If you have used aws cli, they fetch autocomplete results over HTTP.
  2. Another example is git. git checkout and then TAB key would show you all the branches you can checkout with help from tab completions.

I think these features, if implemented would be a huge value add for users.

Some of the challenges could be -

  • not straight forward.
  • requires generating completion files
  • may be separate effort for different shells such as bash, zsh, etc

We can leave it to the users on how they ship autocompletion files to end users as a first step. Next step could be provide way in the app itself so that it installs it's own autocompletion.

@bilal-fazlani
Copy link
Owner Author

bilal-fazlani commented Mar 5, 2020

Imagination

[Command(Description = "Fake git application to demonstrate nested sub-commands. Does NOT interact with git.")]
public class Git
{
    [AutoCompletion]
    public List<String> CheckoutAutoCompletion(String typedCharacters)
    {
        //static
        return new List<string>{"ab", "asd"};
        
        //OR
        //dynamic
        //return GetRemoteData(typedCharacters);
    }

    [Command(Name = "checkout", AutoCompletion = nameof(CheckoutAutoCompletion))]
    public void Checkout()
    {
        
    }
}

@drewburlingame
Copy link
Collaborator

I don't see myself getting to this one anytime soon. As you mentioned, each shell will need its own implementation.

I found a java library called picocli that implements for bash with this class. It doesn't look trivial.

The library has some nice features and documentation. I'm bookmarking as a reference.

@bilal-fazlani
Copy link
Owner Author

It is indeed big work. Thanks for sharing the references 👍

@drewburlingame
Copy link
Collaborator

I realized something that will make a big difference for this feature.

We can utilize the dotnet-suggest tool for this.

You can see from these tests that the features relies on a [suggest:{position}] directive. As long as we support that, we can take advantage of this tool for auto-complete in bash and powershell.

The initial work to support static auto-complete should be fairly simple. We can also support Allowed Values for auto-complete of enums.

The data-driven auto-complete becomes possible but will require some design thought.

@drewburlingame
Copy link
Collaborator

drewburlingame commented Mar 29, 2020

re: dotnet-suggest. I was thinking more about how to add an optional gate for this and thought of something like this

        internal static AppRunner UseAutoSuggestDirective(this AppRunner appRunner, 
            Func<CommandContext, bool> registerWithDotNetSuggestWhen = null)

Devs could then introduce a [register-dotnet-suggest] directive by checking ctx.Tokens.TryGetDirective('register-dotnet-suggest...).

appRunner.UseAutoSuggestDirective(ctx =>
{
  if(ctx.Tokens.TryGetDirective('register-dotnet-suggest`, out _))
  {
    RegisterWithDotNetSuggest(ctx);
  }
});

They could also use that directive to auto-install dotnet-suggest, download the bash shim and the reference in the ~./bash_profile We could document that approach or add it as another feature by providing the delegate. The main issue with adding as a feature is testing Bash & PowerShell. It may be better offered as a Gist.

appRunner.UseAutoSuggestDirective(ctx =>
{
    if(ctx.Tokens.TryGetDirective('register-dotnet-suggest`, out _))
    {
        EnsureDotNetSuggest();
        RegisterWithDotNetSuggest(ctx);
    }
});

public static bool EnsureDotNetSuggest()
{
    InstallDotNetSuggestTool();
    // fallback to embedded resource if cannot download latest from github
    InstallBashShim(latestFromGitHub: true);
    InstallPowerShellShim(latestFromGitHub: true);
}

@bilal-fazlani
Copy link
Owner Author

Yes, I like the gist idea.

@bilal-fazlani
Copy link
Owner Author

bilal-fazlani commented Mar 29, 2020

In case of [debug] & [parse], what arguments/command you pass to CLI, matters. Those additional args are used by the framework.

But in case [register-dotnet-suggest] it seems weird because you would ideally not mix [register-dotnet-suggest] without your actual arguments/command

for example:

dotnet email.dll [register-dotnet-suggest] send bilal@yahoo.com

Instead this it seem more fit for a top level command like

dotnet email.dll register-dotnet-suggest

Also, I think the name can be changed to more abstract and user friendly: install-auto-completion

Another thought -

The choice of whether or not to install auto-suggest should be of end users by default and developers can out-out explicitly. This way users would always see install-auto-suggest in the help if developers don't disable it. If users execute the command install-auto-suggest, and don't have dotnet-suggest tool installed already, we would give them a detailed response on CLI on how they can install it. It can also include a link to our documentation page where they can see even more details in friendly HTML format instead of CLI. The instruction may include directions on

  • how to install dotnet-suggest &
  • how to install the bash or ps shim scripts

We should print the link to our help page even when dotnet-suggest registration was successful because users will still need to install shim scripts

@drewburlingame Thoughts ?

@drewburlingame
Copy link
Collaborator

I agree with your point that calling a directive without a command seems odd. I've wondered about that myself. The way I reason about it is that the app could have a default command, it would look similar. In the end, while it's odd, I don't think it's unreasonable.

It seems more invasive to me though for us to insert a command that will appear in the help menu. I've wondered if it makes sense to a "Utilities" command (renameable) with subcommands for various commands like install-auto-suggest, generate-man-files, etc.

Perhaps the best of both worlds is to provide the classes to do the work and a setting to include the command or directive, but allowing the dev to create their own command if they'd prefer but reuse the logic we have. I think we should directly reference the command-line-api docs since they could update at any time. It would be up to the devs how to reference those for their users.

@bilal-fazlani
Copy link
Owner Author

The way I reason about it is that the app could have a default command, it would look similar.

I did't understand this. Could you please elaborate more?

It seems more invasive to me though for us to insert a command that will appear in the help menu.

It is invasive yes. But I think it's worth it. Here's why:

Be it directive or command or something totally different, I think the users should know that the cli app they just installed supports auto completion and they can enable it by following one or more simple steps. I find this feature a huge value add for users and feel like they will miss out a lot if we keep it disabled or hidden behind a directive by default.

Directives seem perfect for things you have shown me like debug cmdlog and parse. These are very helpful for developers but none of these things are expected to be used by an end user of the app in normal scenarios. You can take MrGitTags for example.

My concern with directives is that, not many end users may be aware of the concept of a directive. I have development experience of 8-9 years and I came to know about directives from you. It may be possible that directives are not as common as commands, flags/options and operands among cli users. The fact that directives don't appear in help adds to that problem. Even if end users are familiar with concept of directives, they don't know what directives are available.. so they will never know that they could enable auto completion. Please correct me if I am wrong here... I just assumed directives are not shown in help.

If not a "command", then anything else really. My core need was just that somehow I didn't want end users to go and read documentation of the app to find out about auto completion. I have observed that, for cli apps, people mostly rely on --help to explore the app instead of visiting the documentation of the cli app.

@drewburlingame
Copy link
Collaborator

The way I reason about it is that the app could have a default command, it would look similar.

I did't understand this. Could you please elaborate more?

if dotnet email.dll had a default command that required no arguments, then dotnet email.dll [parse] is still valid. It's a weak argument. Maybe more to the point, the developer provided documentation told them to type to setup auto-suggest. They don't have to know it's a directive or command or whatever. They'll copy-paste, so I'm not too worried about that.

re: invasive

There's a difference between making it easy for the developer to provide features for the user and by-passing the judgment of the developer to provide a feature we think the user should have.

There may be reasons we don't know why a dev wouldn't want this option displayed. A strong one is that they don't control the feature but they still have to support it because it ships with the library. What happens if we stop working on this library and dotnet-suggest API changes. There'd always be waiting for the next support ticket.

If the dev can control if and where it appears in the help, then we've provided them with the tool, and have not forced it on them. I think that's the golden-spot. ShowInHelp is currently only available or Options so that would need to be moved to IArgumentNode. Adding the command could be an option for enabling the feature but I think it should default to hidden with an option for the devs to show it.

@bilal-fazlani
Copy link
Owner Author

bilal-fazlani commented Mar 31, 2020

if dotnet email.dll had a default command that required no arguments

Ok yes I understand what you mean by this now. I agree users don't have to know what is a directive if all they have to do is copy paste the line. But to do a copy paste, they have to visit the documentation and come across auto completion section and then realise - "ohh there's auto completion too".

You are right a similar issue exists for default command, just reopened issue #28

A strong one is that they don't control the feature but they still have to support it because it ships with the library.

Fair point. But do you think this holds true when we provide a way to disable auto-completion? We don't really want to force any opinion on devs by not making it configurable. We make it configurable and also give a default that we think will be suitable for most devs.

Now we just need to think what would "most" of audience want. That I believe is the key. Would most want to enable or most of the devs want to disable? If you think it's the former is true, then it makes sense to ship it enabled by default and if you think the latter is true then it makes sense to ship it disabled by default. I think it's the former. What do you think?

Adding the command could be an option for enabling the feature but I think it should default to hidden with an option for the devs to show it.

Let's take some more time to think about it. There are some factors that we will need to consider, for example:
When app gets updated and its commands and arguments get changed, added & removed, users would want auto completion to get updated automatically as well.
There could be more than one ways to achieve this. One way could be that developers set an expectation with their users that they need to perform the install-auto-completion every time they upgrade the app.
Another way could be we perform install-auto-completion on every run if it is enabled, so users know nothing about this at all. They don't see install-auto-completion command or directive in help. You raised a point that it may hit performance. Let's run some experiments to how much is the performance hit and then take a call. I can take this up and will posts results here.

@drewburlingame
Copy link
Collaborator

We're on the same page, provide a way to enable the option and provide a sensible default when enabled. I think there are a few extensions for this:

  • appRunner.UseAutoSuggest(); enables the [suggest] command directive
  • appRunner.UseAutoSuggestInstaller(showInHelp: true); adds the command. When showInHelp is true, the command will appear in the root level command help.

re: app updates:

There's nothing we have to do. dotnet-suggest will call our app everytime to get the suggestions. When the app is updated, it will return updated suggestions. The only need to register once per app. If you feel strongly about auto-install, we could add another parameter to UseAutoSuggestInstaller: UseAutoSuggestInstaller(autoInstallOnEveryRun: true). We could document that will add a flag to the apps temp folder to indicate if we've already installed. That check would still happen every run.

@drewburlingame
Copy link
Collaborator

Update: I've been working on this. I've encountered some additional complexity we'll need to handle. When [suggest] is included, it can include a "position" parameter indicating a position in the provided command line. Simple enough. The AutoSuggest middleware will run after the command parser and after all of the token transformations. The token transformations can expand the tokens. For example, if response files are used, we'll replace that token with all the tokens found in the file. This could invalidate the position. We'll need to be able to map the position with the original tokens to the expanded location.

Tokens are mapped to the tokens they came from which can be used for a reverse lookup but we lose precision. If the position falls in the middle of a token that was expanded, we may not be able to determine where the position maps to in the expanded token set.

When no position is given, we'll be fine.

@wayneyaoo
Copy link

Hey bump this up. Thanks for making such a great library! I see auto completion feature in Cocona here. It works by adding a bunch of text (unfortunately I have no idea how that works) to bashrc or zshrc. The mysterious texts are somehow .. embedded inside the library, though I do see generators. Do you think this is a good approach to look into? I mean the way to add autocompletion there is fine, even if it's manual. Compared to dotnet-suggestion, which requires to install an extra tool, this is far more better.

Thank you again!

@drewburlingame
Copy link
Collaborator

I agree adding the text would be better, but I don't have the bandwidth to learn the way to do it for bash. zsh can inherit from bash so I think we'd only need the one. I have a PR that I need to finish documenting that will use dotnet-suggestion.

If someone implemented the bash script, it could call the suggest directive just like dotnet-suggestion does and that would prevent the need to update the script every time the tool is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
CommandDotNet
  
To Do
Development

No branches or pull requests

3 participants