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

feat(cmd): added command to generate basic cmd autocompletion scripts #1240

Merged
merged 6 commits into from
Apr 6, 2020

Conversation

Arqu
Copy link
Contributor

@Arqu Arqu commented Apr 1, 2020

Relevant to #1236

Took a mix of inspiration between cobra docs and kubectl handling of zsh support.
Currently it only handles registered commands and flags, no support for dataset name autocomplete just yet.
The convoluted approach for zsh is due to cobra not supporting "advanced/custom" autocomplete functions and thus would not work for the future dataset name autocompletion (or similar scenarios like peers etc).

@Arqu Arqu self-assigned this Apr 1, 2020
@Arqu
Copy link
Contributor Author

Arqu commented Apr 1, 2020

Depends on #1242 and will also require updates/new flags for some commands to have easily digestable output for shell scripts.

@Arqu Arqu added CLI command line client issues UX Issues that affect User eXperience and may require rethinking how something works labels Apr 2, 2020
@Arqu Arqu linked an issue Apr 2, 2020 that may be closed by this pull request
@Arqu
Copy link
Contributor Author

Arqu commented Apr 2, 2020

Depends on #1247

cmd/completion.go Outdated Show resolved Hide resolved
cmd/completion.go Outdated Show resolved Hide resolved
cmd/completion.go Outdated Show resolved Hide resolved
cmd/completion.go Outdated Show resolved Hide resolved
cmd/completion.go Outdated Show resolved Hide resolved
@Arqu
Copy link
Contributor Author

Arqu commented Apr 2, 2020

Also just to be complete on the evolution of this, here's a list of commands (not exhausitve, but almost) which I'll update and reflect on depending on the case. Flags are still the odd case I don't handle here and not sure if I'll completely cover it in just this PR.

No dependencies:

  • qri connect
  • qri init (intentionally left manual)
  • qri registry (intentionally left manual)

Depends on qri search:

  • qri fetch
  • qri add
  • qri search

Depends on qri list:

  • qri render
  • qri stats
  • qri logbook
  • qri validate
  • qri save
  • qri export
  • qri use
  • qri rename
  • qri remove
  • qri publish
  • qri log
  • qri checkout
  • qri get

Will not cover now (reason):

  • qri config (config args)
  • qri get (structure args)
  • qri fetch (—remote)
  • qri publish (—remote)
  • qri diff (combined completion components, datasets and files)
  • qri restore? (versions, structure args)
  • qri list (peers)
  • qri dag (hidden)
  • qri fsi (hidden)
  • qri (--repo)
  • qri sql (completely different type of autocomplete)
  • qri update (complex)
  • qri peers (complex)
  • qri status (status currently does not behave as expected, additionally it depends on full dataset+location completions)

@Arqu Arqu marked this pull request as ready for review April 3, 2020 21:16
@Arqu Arqu requested review from dustmop and b5 April 3, 2020 21:16
@Arqu
Copy link
Contributor Author

Arqu commented Apr 3, 2020

Another consideration would be to ship pre-generated autocomplete files so you can just source ~/path/to/qri/source_file

For more input refer to the above comment on the specific functionality.
Additional note: search is currently slightly borked but only because it serves results that cloud gives back. Results will inprove as qri.cloud serves better results.

Copy link
Member

@b5 b5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok just ran this, so nice.

I think we'll need a few passes of fitting & cleanup–as you've noted there are additional commands not yet covered by this PR–but those can come in time. As an example qri get ... <tab> lists dataset names (yay!), but kinda obfuscates qri get body <tab>. Don't know if or how we want to handle that scenario.

All that said, wow do completions make life on the CLI easier. The benefits of autocompletion far outweigh any quirks.

o := &AutocompleteOptions{IOStreams: ioStreams}
cmd := &cobra.Command{
Use: "completion [bash|zsh]",
Short: "Generates shell completion scripts",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeping with our CLI Help Style, should be generate shell auto-completion scripts

RunE: func(cmd *cobra.Command, args []string) error {
return o.Run(cmd, args)
},
ValidArgs: []string{"bash", "zsh"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the Annotations field to make this show up in qri --help:

Annotations: map[string]string{
  "group": "other",
}

Short: "Generates shell completion scripts",
Long: `To load completion run

source <(qri completion [bash|zsh])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a $ for anything the user should type, and consider moving these into the Examples field.

@Arqu
Copy link
Contributor Author

Arqu commented Apr 6, 2020

Ok just ran this, so nice.

I think we'll need a few passes of fitting & cleanup–as you've noted there are additional commands not yet covered by this PR–but those can come in time. As an example qri get ... <tab> lists dataset names (yay!), but kinda obfuscates qri get body <tab>. Don't know if or how we want to handle that scenario.

All that said, wow do completions make life on the CLI easier. The benefits of autocompletion far outweigh any quirks.

Yeah, there's still quite a bit of work to do to have it complete, but did my best to document the things that work and that don't.

For the specific example with qri get ... <tab>, I need to get a full list of structure args and then figure out how to serve that in the completion.

Anyways, my intention was to keep the original issue alive and have it document the missing pieces so we can pick it up again later.

@Arqu Arqu merged commit 0845289 into master Apr 6, 2020
@Arqu Arqu deleted the feat_autocomplete branch April 6, 2020 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI command line client issues UX Issues that affect User eXperience and may require rethinking how something works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants