cobra icon indicating copy to clipboard operation
cobra copied to clipboard

Completion support for Nushell

Open ayax79 opened this issue 2 years ago • 97 comments

Completion support for Nushell. Besides the tests included, I have also tested against minikube.

ayax79 avatar Nov 15 '22 20:11 ayax79

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Nov 15 '22 20:11 CLAassistant

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Nov 15 '22 20:11 github-actions[bot]

Hey @ayax79 this exciting! I have never used nushell but I have now installed it on my Mac.

I see that this PR generates a script containing the completions. However, this is no longer the approach used by Cobra. Instead, the program, through Cobra, provides the completions itself through the hidden __complete command. The nushell script should then call prog __complete <all other args on the command-line> to obtain the list of completions and it should use them as appropriate for nushell. You can test out the __complete command manually by doing things like prog __complete '' or prog __complete -- and such.

This approach means that we have centralized all the logic to generate completions and coded it in Go (in Cobra's completions.go file). Then Cobra generates a shell script for each supported shell which uses the __complete command to get the completions.

Following this approach would be your first step. You can look at the completion scripts for any of the other shells to see how this is done. These scripts are in files named for the shell they support.

Eventually, the nushell script should also support the ShellCompDirectives, which you will notice in the other scripts.

Feel free to ask for more info if needed.

marckhouzam avatar Nov 16 '22 03:11 marckhouzam

Hey @marckhouzam,

Thanks for looking at this and thanks for the feedback!

Nushell completions quite a bit differently than completions in other shells I have used (zsh). nushell_completions.go doesn't generate a script for the completions, it generates something more akin to a header file. each "export extern " maps to an external, non-nushell command. An example would be export extern "minikube start" It is telling nushell about the 'minikube start' sub-command and describing the parameters it accepts.

For example, see the minikube-completions.nu I generated. minikube-completions.nu.txt

You can load it into nushell via (added .txt to get it to upload): use minikube-completions.nu.txt *

I'll dig into the new model, but I would love any implementation advice.

ayax79 avatar Nov 16 '22 05:11 ayax79

We had a similar situation with Fish where normal completions are a long list of choices. So in our case we have one generic completion entry which calls a function that calls prog __complete ... and processes the results.

The nushell doc talks about custom completions which seem like an avenue to investigate. For example it shows:

export extern "git push" [
    remote?: string@"nu-complete git remotes"
...

which calls git remotes to get the list of completions.

marckhouzam avatar Nov 16 '22 11:11 marckhouzam

An important advantage of Cobra's approach is that using the __complete command in your nushell script will automatically support dynamic completions provided by the program using Cobra (which your solution doesn't). Please see details here: https://github.com/spf13/cobra/blob/main/shell_completions.md#dynamic-completion-of-nouns

marckhouzam avatar Nov 16 '22 11:11 marckhouzam

@rsteube the author of (carapace)[https://github.com/rsteube/carapace] chimed in with some good feedback on the nushell discord that I will include too. As already commented in the issue it is better to take the external_completer (https://www.nushell.sh/blog/2022-09-06-nushell-0_68.html#external-completions-experimental-herlon214-rsteube) approach for this. You can grab some insights from https://github.com/rsteube/carapace/blob/master/internal/shell/nushell/action.go if it's any help. For the meantime i got a custom completer for minikube and kubectl at https://github.com/rsteube/carapace-bin . Completion of cobra based commands can also be bridged if those don't work well for you: https://rsteube.github.io/carapace-bin/spec/bride.html .

ayax79 avatar Nov 16 '22 17:11 ayax79

I have played around with a few options and haven't found anything that works well utilizing __command.

This works for only the first level for commands and breaks flags:

def 'nu-complete minikube' [] {
	minikube __command
}

export extern minikube [
	command?: string@'nu-complete minikube'
]

Screenshot_minikube_level0

Broken help flag: Screenshot_minikube_broken_help

To access a flag you have to bypass nushell via:

^minikube --help

By adding another level:

def 'nu-complete minikube start' [] {
    minikube start __command
}

export extern 'minikube start' [
	command?: string@'nu-complete minikube start'
]

This overrides the completion list and removes the description (unless it is manually added as comment). It also breaks any flags.

The other option is to utilize the external completer support. This however only provides an option for one global fallback completer. This how carapace-bin works with nushell.

I'll poke around more, but it seems like I am may be running out of options.

ayax79 avatar Nov 18 '22 20:11 ayax79

Thanks for continuing the investigation @ayax79.

In the above comment you typed __command everywhere, but I assume you meant __complete? Also be aware that the format is prog __complete <other args>, so the __complete command should always be the first argument to the program.

It is also important to pass the current command-line arguments that the user typed. So, if the user typed minikube start --<tab> to complete flags, the way to get the completions is to call minikube __complete start --.

Finally, if the last character on the command-line before the user pressed TAB is a space, you must pass an extra empty argument. So, if the user typed minikube start <TAB> (notice the space after start) you need to call minikube __complete start "". This complexity allows cobra to tell the difference between the user typing minikube start<TAB> which is request to complete the start argument, versus minikube start <TAB> which is a request to complete the argument after start.

You can find documentation about all that here: https://github.com/spf13/cobra/blob/main/shell_completions.md#debugging

marckhouzam avatar Nov 18 '22 20:11 marckhouzam

Yeah you'll definitely have no success with the extern approach, it won't allow unknown flags. Sorry if it wasn't clear before, but you can do some nu scripting magic to support mutliple completers - it's just bad in regards to "registering" them.

let external_completer = {|spans| 
  {
    $spans.0: { } # default
    kubectl: { kubectl __complete ($spans | into df | slice 1 1000) | handleOutput }
    minikube: { minikube __complete ($spans | into df | slice 1 1000) | handleOutput }
  } | get $spans.0 | each {|it| do $it}
}

rsteube avatar Nov 18 '22 20:11 rsteube

I finally got some more time to work on this. I also spent some time digging into the cobra completions and the nushell external completions code .

This seems to work, although it needs some more testing. There might be some weirdness around mixing flags and commands:

# Executes a cobra apps __complete cmd with the following span
export def exec_cobra_complete [
  cmd: string,  # The cobra command (e.g. kubectl)
  spans: list,  # The list of spans
 ] {
  # skip the first entry in the span (the command) and join the rest of the span to create __complete args
  let cmd_args = ($spans | skip 1 | str join ' ') 

  # If the last span entry was empty add "" to the end of the command args
  let cmd_args = if ($spans | last | str trim | is-empty) {
    $'($cmd_args) ""'
  } else {
    $cmd_args
  }

  # The full command to be executed
  let full_cmd = $'($cmd) __complete ($cmd_args)'

  # Since nushell doesn't have anything like eval, execute in a subshell
  let result = (do -i { nu -c $"'($full_cmd)'" } | complete)

  # Create a record with all completion related info. 
  # directive and directive_str are for posterity
  let stdout_lines = ($result.stdout | lines)
  {
    completions: ($stdout_lines | drop | lines | parse "{value}\t{description}")
    directive_str: ($result.stderr)
    directive: ($stdout_lines | last)
  }
}

# Execute the __complete command and only return completions
export def get_cobra_completions [
  cmd: string,  # The cobra command (e.g. kubectl)
  spans: list,  # The list of spans
] {
  # return just the completions from the result. 
  (exec_cobra_complete $cmd $spans).completions
}

let external_completer = {|spans|
  {
    $spans.0: { }
    kubectl: { get_cobra_completions 'kubectl' $spans }
    minikube: { get_cobra_completions 'minikube' $spans }
  } | get $spans.0 | each {|it| do $it}
}

As @rsteube mentioned, registration will be a little bit odd. I think that can be documented in commends for the completion generation though. It will require adding the that code block to the nushell config file ($nu.config-path) and updating config.external_completer entry:

let-env config = {
    external_completer: $external_completer
}

One nice thing is that for other cobra commands, only the external_completer block will need to be updated.

@marckhouzam, if this approach looks good, I'll update nushell_completer.go with it.

ayax79 avatar Nov 23 '22 19:11 ayax79

Thanks @ayax79. I was able to test the above script and things look promising. However, we need to come up with a solution to enable completion for more than one tool using Cobra. Here is where I think it needs some thought.

I'm assuming that each tool, myprogram for example, will generate the above script ending with

let external_completer = {|spans|
  {
    $spans.0: { }
    myprogram: { get_cobra_completions 'myprogram' $spans }
  } | get $spans.0 | each {|it| do $it}
}

The problem is that if I have multiple tools using this, only the last sourced script will work as each one sets the same external_completer variable. What we do in Cobra is prefix each variable and function in the script by <programName>_. In this case the script would set variables such as myprogram_external_completer.

After that, we need to figure out a way to have the one

let-env config = {
    external_completer: $external_completer
}

manage to include all tools that have cobra completions.

marckhouzam avatar Nov 24 '22 16:11 marckhouzam

If we took this approach, it would still be possible to wrap the cobra completer and chain completers together -- it would be up to the user to figure how they wanted to handle it.

The configuration itself isn't changeable once the shell is loaded. Nushell works much more like a compiled language. Dynamic loading of resources and execution isn't really possible. The two phase execution approach is part of the reason why nushell is so fast, but it does create some awkwardness. You can see how I got around this, by creating a subshell to execute the command I assembled:

 # Since nushell doesn't have anything like eval, execute in a subshell
 let result = (do -i { nu -c $"'($full_cmd)'" } | complete)

What I really don't like about this approach is that nushell's environment has to load to execute the command (I think, need to verify). It might be feasible to do something kind of like execute completions from a directory, but it could get expensive:

let external_completer = {|spans|
    let completion_command = $"($env.COMPLETION_DIR)/($spans.0).nu ($spans | str join ' ')"
    nu -c $completion_file 
}

Using something like carapace-bin is the best use of external_completers, because carpace itself is plugable.

ayax79 avatar Nov 24 '22 19:11 ayax79

Not knowing anything about nushell, I'm having trouble following the subtleties.
If you could describe the steps the user would take to get the completions to work for multiple tool, we could see if it is something that we can move forward with.

If we can reach a solution where any one tool can generate some nushell completion script and the user could add something to their configuration file to setup any number of such scripts, we would have something sufficient.

marckhouzam avatar Nov 24 '22 20:11 marckhouzam

I simplified the nushell external configurator to this:

# An external configurator that works with any cobra based
# command line application (e.g. kubectl, minikube)
let cobra_configurator = {|spans| 
  let cmd = $spans.0

  # skip the first entry in the span (the command) and join the rest of the span to create __complete args
  let cmd_args = ($spans | skip 1 | str join ' ') 

  # If the last span entry was empty add "" to the end of the command args
  let cmd_args = if ($spans | last | str trim | is-empty) {
    $'($cmd_args) ""'
  } else {
    $cmd_args
  }

  # The full command to be executed
  let full_cmd = $'($cmd) __complete ($cmd_args)'

  # Since nushell doesn't have anything like eval, execute in a subshell
  let result = (do -i { nu -c $"'($full_cmd)'" } | complete)

  # Create a record with all completion related info. 
  # directive and directive_str are for posterity
  let stdout_lines = ($result.stdout | lines)
  let result = ({
    completions: ($stdout_lines | drop | lines | parse "{value}\t{description}")
    directive_str: ($result.stderr)
    directive: ($stdout_lines | last)
  })

  $result.completions
}

The process would be:

  1. Run <cmd> completion nushell to generate the above block
  2. Add the above block to the $nu.config-path (location of the nushell config file)
  3. Configure the entry external_configurator under config to be $cobra_configurator

There will be no need to change the configuration for any subsequent cobra based app. The above configuration will work with any cobra based app without any further change.

ayax79 avatar Nov 26 '22 08:11 ayax79

Wow! After sourcing the script in the latest comment and then doing

let-env config = {external_completer: $cobra_configurator}

I was able to do completion for helm and kubectl!

One question with this approach: are we going to break completion for any non-cobra program that uses the external_completer?

@ayax79 Could you update the PR with the new script and then I can start testing more thoroughly? Also, there are the shell completion directives we'll have to try to support.

One thing to investigate is that flag value completion does not seem to work:

$ kubectl --namespace [tab]
NO RECORDS FOUND

We should be getting a list of namespaces.

But this is a great start!

marckhouzam avatar Nov 26 '22 19:11 marckhouzam

The external completer is a fallback. Completions using externs will load first. If someone wants to use more than one external completer they could write a completer that wraps two:

let chaining_completer = {|spans|
  let completions = (do $cobra_completer $spans)
  let completions = if ($completions | is-empty) {
    (do $other_completer $spans)
  } else {
    $completions
  }
  $completions
}

I'll poke around with the flag completions. It should work the same as anything else.

ayax79 avatar Nov 26 '22 19:11 ayax79

There was a bug in the line parsing causing it to drop entries that weren't exactly {value}\t{description}. Anything without a description was getting dropped. I changed to parse via regex. The only thing that might get dropped with this one is values (commands/flags/options) that have particularly exotic characters, I am matching [\w-.:+]. This is the corrected version:

# An external configurator that works with any cobra based
# command line application (e.g. kubectl, minikube)
let cobra_configurator = {|spans| 
 
  let cmd = $spans.0

  # skip the first entry in the span (the command) and join the rest of the span to create __complete args
  let cmd_args = ($spans | skip 1 | str join ' ') 

  # If the last span entry was empty add "" to the end of the command args
  let cmd_args = if ($spans | last | str trim | is-empty) {
    $'($cmd_args) ""'
  } else {
    $cmd_args
  }

  # The full command to be executed
  let full_cmd = $'($cmd) __complete ($cmd_args)'

  # Since nushell doesn't have anything like eval, execute in a subshell
  let result = (do -i { nu -c $"'($full_cmd)'" } | complete)

  # Create a record with all completion related info. 
  # directive and directive_str are for posterity
  let stdout_lines = ($result.stdout | lines)

  let $completions = ($stdout_lines | drop | parse -r '([\w\-\.:\+]*)\t?(.*)' | rename value description)

  let result = ({
    completions: $completions
    directive_str: ($result.stderr)
    directive: ($stdout_lines | last)
  })

  $result.completions
}

I will have an updated pull request shortly.

ayax79 avatar Nov 26 '22 22:11 ayax79

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Nov 27 '22 21:11 github-actions[bot]

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Nov 27 '22 21:11 github-actions[bot]

I just found a nasty bug.

When running things vim, it locks up because vim interprets vim __complete as a valid command and never returns.

ayax79 avatar Nov 27 '22 22:11 ayax79

I think I am going to have to put a whitelist in for commands to execute the completer for:

# A list of cobra that completion will be attempted for.
# Add new apps to this list to enable completion for them.
let cobra_apps = ["kubectl", "minikube"]

# An external configurator that works with any cobra based
# command line application (e.g. kubectl, minikube)
let cobra_completer = {|spans| 
  let cmd = $spans.0

  if not ($cobra_apps | where $it == $cmd | is-empty) {
    let ShellCompDirectiveError = 1
    let ShellCompDirectiveNoSpace = 2
    let ShellCompDirectiveNoFileComp = 4
    let ShellCompDirectiveFilterFileExt = 8
    let ShellCompDirectiveFilterDirs = 16
   
    let last_span = ($spans | last | str trim)

    # skip the first entry in the span (the command) and join the rest of the span to create __complete args
    let cmd_args = ($spans | skip 1 | str join ' ') 

    # If the last span entry was empty add "" to the end of the command args
    let cmd_args = if ($last_span | is-empty) {
      $'($cmd_args) ""'
    } else {
      $cmd_args
    }

    # The full command to be executed with active help disable (Nushell does not support active help)
    let full_cmd = $'($cmd)_ACTIVE_HELP=0 ($cmd) __complete ($cmd_args)'

    # Since nushell doesn't have anything like eval, execute in a subshell
    let result = (do -i { nu -c $"'($full_cmd)'" } | complete)

    # Create a record with all completion related info. 
    # directive and directive_str are for posterity
    let stdout_lines = ($result.stdout | lines)
    let directive = ($stdout_lines | last | str trim | str replace ":" "" | into int)
    let completions = ($stdout_lines | drop | parse -r '([\w\-\.:\+\=]*)\t?(.*)' | rename value description)

    # Add space at the end of each completion
    let completions = if $directive != $ShellCompDirectiveNoSpace {
      ($completions | each {|it| {value: $"($it.value) ", description: $it.description}})
    } else {
      $completions
    }

    if $last_span =~ '=$' {
      # return flag as part of the completion so that it doesn't get replaced
      $completions | each {|it| $"($last_span)($it.value)" }
    } else if $directive == $ShellCompDirectiveNoFileComp {
      # Allow empty results as this will stop file completion
      $completions
    } else if ($completions | is-empty)  or  $directive == $ShellCompDirectiveError {
      # Not returning null causes file completions to break
      # Return null if there are no completions or ShellCompDirectiveError 
      null
    } else {
      $completions
    }
  } else {
    null
  }
}

I don't think it will be too onerous. The workflow will be:

If you haven't setup cobra_completer

  1. edit config.nu
  2. add generated contents
  3. set external_completer = cobra_completer
  4. start new shell

If you have setup cobra_completer

  1. Update the cobra_apps array to contain your app, for example:
let cobra_apps = ["kubectl", "minikube", "helm"]
  1. restart shell

ayax79 avatar Nov 27 '22 22:11 ayax79

I think I am going to have to put a whitelist in for commands to execute the completer

For other shells we don't use a "global" completer but instead have to register each program individually. So if kubectl generates the completion script, it also register kubectl (and only kubectl) to use the functions in that script.

If nushell completions don't work like that then your allow-list idea sounds like the right approach. I wonder if we could automate adding a program name to the list in the script itself?

marckhouzam avatar Nov 27 '22 22:11 marckhouzam

@rsteube if you have any guidance for us don't hesitate to chime in and correct us.

marckhouzam avatar Nov 27 '22 22:11 marckhouzam

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Nov 27 '22 23:11 github-actions[bot]

We could certainly automate adding a line like: let cobra_apps = ($cobra_apps | append "newapp") to below the initial cobra_apps definition in the config.nu

ayax79 avatar Nov 27 '22 23:11 ayax79

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Nov 28 '22 21:11 github-actions[bot]

@ayax79 Sorry for the delay. I'm having trouble finding enough time to do a thorough review, so let's start with the last remaining points I noticed.

1- First, things are looking really great! I'm really impressed. Excellent job!

2- I'm still getting the below weird behaviour. I haven't had time to look into it myself. Notice the NO RECORDS FOUND when I just type k as a prefix for the namespaces.

I'm getting a weird behaviour where no records are found when I think they should:

$ kubectl get ns
NAME              STATUS   AGE
default           Active   23d
kube-system       Active   23d
kube-public       Active   23d
kube-node-lease   Active   23d
$ kubectl get namespaces k[tab]
NO RECORDS FOUND
$ kubectl get namespaces ku[tab]
kube-system
kube-public
kube-node-lease

3- The fuzzy matching works well! Super nice! However, it is a little too fuzzy 😄 . See below:

  1. New (advanced) requirement that I forgot about: the __complete command may return completions that don't match the last argument the user typed. [...] The filtering rules in the script should be as follows: 1- try to match on prefix. If any completions start with what the user has typed, then only those completions should be kept. 2- if no completion choices match on prefix, then we should look for any completions that contain what the user typed

When I test, it seems the completions don't first match on prefix, but immediately match on "contains". Is it possible to first match on prefix and only if there are no matches to use "contains"? It would make the following case better.

Say I have two namespaces "test" and "mytest". If the user does helm -n te<tab> the completion should complete immediately to test since it is the only one left after matching on the prefix te. Currently, both completions continue to match, so the user much select the test menu or type out test explicitly.

4- I was testing the ShellCompDirectiveNoSpace and at least when the completion ends with a / things don't behave properly. For example, the completion experience is bad when I have this output:

helm __complete install test ''                                                        12/11/2022 09:29:37 PM
bitnami/	https://charts.bitnami.com/bitnami
origami/	https://charts.bitnami.com/bitnami
artifact-hub/	https://artifacthub.github.io/helm-charts/
mytest/	https://charts.bitnami.com/bitnami
test/	https://charts.bitnami.com/bitnami
oci://	Chart OCI prefix
https://	Chart URL prefix
http://	Chart URL prefix
file://	Chart local URL prefix
./	Relative path prefix to local chart
/	Absolute path prefix to local chart
:2
Completion ended with directive: ShellCompDirectiveNoSpace

I can provide more info about this when I have a bit more time.

I'll stop here but will try to find time soon.

We're almost there!

marckhouzam avatar Dec 12 '22 02:12 marckhouzam

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Dec 15 '22 21:12 github-actions[bot]

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

github-actions[bot] avatar Dec 16 '22 17:12 github-actions[bot]