stratis-storage / stratis-cli

CLI for the Stratis project
https://stratis-storage.github.io
Apache License 2.0
105 stars 21 forks source link

For getting values for autocompletion we could possibly skip the CLI layer #447

Open mulkieran opened 6 years ago

mulkieran commented 6 years ago

It would be nice if we could invoke the functions that get the values we need more directly, instead of parsing them back out of the CLI. We would probably have to export auto-complete functions from the CLI, but these would be easy to construct.

I'm not sure how they would interact with bash, but they would just print out the values desired by the auto-completion, presumably.

That way we avoid parsing unstable CLI output to get our values.

mulkieran commented 4 years ago

The idea was that there would be a CLI subcommand, "auto", used by auto-completers. This would have various commands to get the auto-completion data printed out. But I don't see why the CLI should do this. It's probably best done as a separate utility, so I'm going to move it to project-wide and hope the idea catches on w/ somebody.

mulkieran commented 4 years ago

Actually, there is maybe a very good idea for the CLI. Add a new flag: --suggest and if this flag is set, let the action calculate the suggested arguments and print them out. The problems would all be with persuading the parser to call the action method even if the full quota of arguments wasn't there.

Then, there would be a standard exception to raise in the event that there was no code to calculate the suggestion for the particular partial command.

We would catch it, and make a clear statement indicating that contributions would be appreciated.

Normal users would probably appreciate being able to use it, and tab-completion implementors for various shells could use it to. It might attract contributions.

mulkieran commented 4 years ago

There is something called argcomplete, but as AFAICT, it is not contextual: https://pypi.org/project/argcomplete/. On a rename of a filesystem, for example, it would not suggest the subset of filesystems appropriate for a designated pool. Three screens into a PyPI search for tab-completion I could not find anything else.

carzacc commented 4 years ago

There is something called argcomplete, but as AFAICT, it is not contextual: https://pypi.org/project/argcomplete/. On a rename of a filesystem, for example, it would not suggest the subset of filesystems appropriate for a designated pool. Three screens into a PyPI search for tab-completion I could not find anything else.

Back when I first wrote the Bash completion script, you had already suggested argcomplete. The reason why I preferred implementing a regular Bash completion script is that, even though argcomplete is very easy to setup, I tested it and it was very slow. IMHO the user shouldn't have to wait a few seconds just to get a suggestion for something simple that doesn't change and can be hardcoded in a real completion script.

Regarding skipping the CLI layer, the only things the Bash completion script (I haven't looked at the fish and zsh ones) gets from the Stratis CLI are the pool and fs names, and that could be made quicker by implementing a faster CLI tool that only does those two things, which wouldn't be too hard to do after all.

daobrien commented 4 years ago

I don't know if this comment belongs here or not (first time user of Stratis) so bear with me. Auto-complete sort of works for stratis pool but it doesn't offer add-data or add-cache as options. The other four subcommands are offered. Could be related to the dash in the subcommand name?

carzacc commented 4 years ago

I don't know if this comment belongs here or not (first time user of Stratis) so bear with me. Auto-complete sort of works for stratis pool but it doesn't offer add-data or add-cache as options. The other four subcommands are offered. Could be related to the dash in the subcommand name?

What shell are you using? I wrote the Bash script and that behavior isn't what I'm getting.

daobrien commented 4 years ago

I don't know if this comment belongs here or not (first time user of Stratis) so bear with me. Auto-complete sort of works for stratis pool but it doesn't offer add-data or add-cache as options. The other four subcommands are offered. Could be related to the dash in the subcommand name?

What shell are you using? I wrote the Bash script and that behavior isn't what I'm getting.

# bash --version
GNU bash, version 4.4.19(1)-release (x86_64-redhat-linux-gnu)

A colleague suggested there might be a missing autocomplete file under /etc/bash_completion.d/ but I'm not familiar with how it works. This is on RHEL8 btw.

mulkieran commented 4 years ago

@daobrien : If you're running on RHEL8, your version is a bit older than on Fedora. Please report your stratis-cli version; that ought to provide the reason behind the different behaviors.

carzacc commented 4 years ago

@daobrien : If you're running on RHEL8, your version is a bit older than on Fedora. Please report your stratis-cli version; that ought to provide the reason behind the different behaviors.

The latest stratis-cli version RHEL8 is running (1.0.4 according to DistroWatch) does not have the latest changes made to the script but it should give add-data and add-cache as options, or at least it works with the script from the corresponding tag here on GitHub.

daobrien commented 4 years ago

stratis-cli-1.0.2-1.el8.noarch stratisd-1.0.3-1.el8.x86_64

stratis pool --help lists all six subcommands but tab completion omits add-data and add-cache

carzacc commented 4 years ago

stratis-cli-1.0.2-1.el8.noarch stratisd-1.0.3-1.el8.x86_64

stratis pool --help lists all six subcommands but tab completion omits add-data and add-cache

Oh, that's why! The script was changed from completing to just add to add-data and add-cache in version 1.0.3!

daobrien commented 4 years ago

ok, so it's not actually a bug. RHEL8 just hasn't caught up yet. Thanks a lot.

mulkieran commented 1 month ago

We should consider exploring the use of argcomplete.