Better completions

[rsteube]

As discussed in #360 this adds consistent dynamic completion for bash, elvish, fish, oil, powershell, xonsh and zsh.
Left the current completion command untouched as a couple minor issues are expected when used in different environments.
Completion script is generated by calling gh _carapace [shell] (shell is optional and will be detected by parent process name).
Added Dockerfile/docker-compose with the various shells to test the completion (needs GITHUB_TOKEN in .env).

Got some documentation about carapace at https://rsteube.github.io/carapace/ (though still very basic and needs to be updated for the recent changes).

Working

• repo completion (uses search api which needs at least two entered characters to deliver results)
• label completion (cached)
• alias completion
• assignee/mention/reviewer completion
• issue/pr completion
• gist completion
• secret completion
• release/asset completion
• config completion
• auth scope completion
  ...

Missing:

• milestone completion
• github enterprise related stuff
• completion of values within quotes (at least not yet in all shells)
• directly invoking api instead of using exec.Command
  ...

#bash
source <(gh _carapace)

# elvish
eval (gh _carapace|slurp)

# fish
gh _carapace | source

# oil
source <(gh _carapace)

# powershell
Set-PSReadlineKeyHandler -Key Tab -Function MenuComplete
gh _carapace | Out-String | Invoke-Expression

# xonsh
COMPLETIONS_CONFIRM=True
exec($(gh _carapace))

# zsh
source <(gh _carapace)

fix #360
related #695
related #1128
related #1775
related #2471

[rsteube]

will squash the commits after review changes

[kvz]

Yes! This (specifically alias completion) is the thing that will make gh the perfect tool for me ✨ - was about to report an issue but found a fix was there already. Thank you for pushing this @rsteube

[rsteube]

@kvz got prebuilt binaries at https://github.com/rsteube/gh/releases if you want to try it out

[mislav]

@rsteube Thank you for keeping this branch updated! This is definitely one big PR, and I ask for your patience while we review and make a decision on the changeset. 🙇

[rsteube]

@mislav Yes, that is not unexpected. Took some shortcuts here and there as a couple of things weren't easy to get working. You probably also want to have a look at carapace itself to decide if it is something you want to add as dependency or not.

Some sidenotes:

• recently simplified the completion script generation so most of the logic is provided by the application itself (invoked on each <TAB>)
• with environment variable CARAPACE_LOG=1 set some basic logging can be found at /tmp/carapace/gh.log
• had to redo the repo flag parsing as a more lenient version is needed for completion (e.g. partially entered / used outside git repo)
• it is also why the command is passed down to the action functions as different issues/pr's should be completed when the repo flag value is changed

[mislav]

@rsteube How does Carapace compare to using Cobra's ValidArgsFunction and RegisterFlagCompletionFunc?

[rsteube]

@mislav Have to look at it myself as i didn't keep much track of it for a while.
When i derived from what cobra provides dynamic completion wasn't possible yet and development was rather stuck.
I can see some similarities and common strategy, don't know about edge cases but basic dynamic completion seems to work.

ValidArgsFunction correlates to PositionalCompletion and RegisterFlagCompletionFunc to FlagCompletion.
The returned string slice supports descriptions in fish style (\t delimited) but seems there is no distinction between display values and the ones actually being inserted (e.g. when completing a path you only want to show the segment being completed without the full path prefix).
This is also what enables separate partial completion with ActionMultiParts in carapace, but i don't see this too relevant for cli.
The basic dynamic completion for cli should be possible at the moment with cobra itself as well if you prefer that, just not for all shells yet (bash, fish and zsh work i think) and a bit more limited and inconsistent.

Carapace just has a few more features and some issues seen in the cobra completion fixed:

• dynamic completion for bash, elvish, fish, oil (this ones still some issues due to oil itself), powershell, xonsh and zsh
• completions of list of values as in the label flag (each label separately and uniquely)
• caching so that the completion of labels doesn't need to retrieve these for each list element
• util functions to better reuse completions (merge, filter, nesting)
• descriptions in bash
• mechanism to show an error message during completion

Things have gotten pretty stable with the features working rather consistently in each shell in carapace so i'd say it's still a bit ahead though.

[rsteube]

Ok this is interesting, since the api is so similar adding the carapace completions to cobra isn't too hard 😄 :

func (c Carapace) FlagCompletion(actions ActionMap) {
	for name, action := range actions {
		if flag := c.cmd.LocalFlags().Lookup(name); flag == nil {
			fmt.Fprintf(os.Stderr, "unknown flag: %v\n", name)
		} else {
			actionMap[uid.Flag(c.cmd, flag)] = action
			c.cmd.RegisterFlagCompletionFunc(name, func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
				rawValues := action.Invoke(Context{Args: args, CallbackValue: toComplete}).rawValues
				result := make([]string, len(rawValues))
				for index, r := range rawValues {
					if r.Description != "" {
						result[index] = fmt.Sprintf("%v\t%v", r.Value, r.Description)
					} else {
						result[index] = r.Value
					}
				}
				return result, cobra.ShellCompDirectiveDefault
			})
		}
	}
}

Will have a look at what it can do by now, but i can already see some issues.

[mislav]

@rsteube Thanks for continuing to work on this. Due to the size of this change (3000 LOC!) and the tight coupling of many pieces of our codebase to a relatively young library (Carapace), I do not think that we will ever merge this as-is, but I keep perusing your code and I learn something new along the way every time. I especially like seeing how you've added completions for flag values based on data backed by the API. ✨

I've recently made a tiny PR to add completions for the --json flag using Cobra builtin functionality and it worked well: #3628

I'm thinking that an incremental approach like that, backed by a framework that we already use (Cobra), would work better for us in the long run. Based on your experience so far, do you see any limitations with that approach?

[rsteube]

@mislav Yes i expected that we would have to split this up due to the size, it was just simply easier to keep track of the changes and test it.
Would've been pretty easy though as most of it are just repetitive additions.

I think for the most part it will work quite alright with cobra's completion but from what i could see when i tested it there are still a couple of issues with it.

Limitations you might encounter (as i did during the development of carapace):

• no completion yet for elvish, oil, xonsh
  Most use bash, fish or zsh anyway though so that's not too much of a problem.
• incomplete dynamic value completion depending on the shell
  Cobra initally generated native shell completion scripts without callbacks to the application so this is still relatively new
  and last time i checked not yet working in each shell (or at least with some issues).
• no descriptions in bash
• inconsistent behaviour depending on the shell
  Cobra relies on shell provided functions like file/directory filtering which in my experience have different behaviour between the shells.
  This also means that it can't be used in a multiparts completion (e.g. --config {key-completion}={folder-completion}).
• no native multiparts support
  Like with the json flag to complete a comma separated list you have to create your own algorithm to split and prefix the the completion values.
  From what i can see cobra does not yet distinguish between the value being inserted and the one being displayed so you will end up with pretty long display values.
  So instead of

  gh issue list --json 'assignees,author,clos'<TAB>
  // closed
  // closed at

  you will have:

  gh issue list --json 'assignees,author,clos<TAB>
  // assignees,author,closed
  // assignees,author,closed at

  You might also want to filter already entered values to complete a unique list.
• no merge, filter, prefix, suffix util functions
  To be able to reuse completion code you will need some util functions, like merging pull requests with branches in pr/checks).
• conditional completion
  Command and args are passed to the RegisterFlagCompletion function so this shouldn't be a problem (e.g. listing issues for the repository from the repo flag).
• alias completion (the ones added by gh config)
  Seems command structure is still rendered to some of the shell scripts so these would need to be updated/regenerated for this to work.
• default completion
  Not configured completion defaults to file completion, so this has to be explicitly disabled where it isn't appropriate (ShellCompDirectiveNoFileComp).
• special characters and values with space
  Major pain as not every shell escapes special characters so this has to be handled by the completion script and as far as i can see there are still some issues.
• values enclosed in quotes
  Another pain as values containing special characters are best to just be enclosed in quotes, but the completion script needs to handle unmatched quotes (double the fun when you got qotes within quotes):

  gh issue list 'assi<TAB>

  In some shells it also has to handle word splitting (part of the command line to be replaced), which might be problematic since a simple split on space is not enough.
  This is even worse in bash where the word split characters can be configured (duh!) and you have to remove the prefix from your values (e.g comma separated list).
• caching
  Since completion involves hitting the github api caching is pretty helpful to keep responses low.
  E.g. completing the list of labels where each segment would cause an api call (bash is even worse and pretty much invokes it with each TAB).
• error message
  cobra.ShellCompDirectiveError can abort the completion but not sure if an error message can be set.
  This is pretty useful to tell the user what went wrong like github.com not being reachable. (pretty easy though by just returning two pseudo completion values)
• direct sourcing of shell scripts
  At least for zsh this was (intentionally) not possible. Not much of a problem but a reoccurring issue as users expect this to work.
  Reason for this is that sourcing many script slows down shell startup (unknown to many) but this can be circumvented when sourced lazily (see lazycomplete)).

Nothing too serious though, but expect fixes regarding this to take a while in cobra.

Also be sure to limit the requests to the graphql api to return only what you need as this has quite an impact on the completion delay.
Oh and test completion outside of a git repository, that was quite a problem in the beginning (repo flag parsing also needs to be a bit more lenient).

I will probably transform this to a standalone completer then where the completion is provided by a separate binary like here. Actually this is already working as is, but does not make too much sense to bundle the whole application for which it provides the completion.

[rsteube]

@mislav moved it here as standalone completer: https://github.com/rsteube/carapace-bin/tree/master/completers/gh_completer/cmd

[mislav]

Mega-thanks for all this!

• no descriptions in bash

But bash doesn't have native support for descriptions, right? I appreciate you being to add them, but I feel that if people want richer shell completions, they should move to a shell that offers richer completion support (e.g. zsh, fish).

• caching

This is a great point. We definitely don't want API-backed shell completions to hit the API repeatedly. 👍