-
Notifications
You must be signed in to change notification settings - Fork 29
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
Comments
github url for auto-complete library : https://github.com/tonerdo/readline |
Change of plans: Here's an example: https://github.com/iridakos/bash-completion-tutorial |
The feature is being developed here: https://github.com/bilal-fazlani/commanddotnet/tree/autocomplete-new |
I wonder if we can tap into Suggestions from System.CommandLine https://github.com/dotnet/command-line-api/wiki/Features-overview And https://docs.microsoft.com/en-us/dotnet/core/tools/enable-tab-autocomplete |
Another nugget for thought. Bash completion supports the keywords |
I had some vision about this feature when I was actively developing on this repo. Static out of the box autocomplete
Example:
Data driven autocomplete
Examples:
I think these features, if implemented would be a huge value add for users. Some of the challenges could be -
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. |
Imagination
|
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. |
It is indeed big work. Thanks for sharing the references 👍 |
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 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. |
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 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 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);
} |
Yes, I like the gist idea. |
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 for example: dotnet email.dll [register-dotnet-suggest] send [email protected] 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: 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
We should print the link to our help page even when @drewburlingame Thoughts ? |
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. |
I did't understand this. Could you please elaborate more?
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 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 |
if 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. |
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
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?
Let's take some more time to think about it. There are some factors that we will need to consider, for example: |
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:
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: |
Update: I've been working on this. I've encountered some additional complexity we'll need to handle. When 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. |
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! |
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. |
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.
The text was updated successfully, but these errors were encountered: