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

Should I expect completion of nested command's flags to work? #1916

Closed
dearchap opened this issue May 25, 2024 Discussed in #1915 · 31 comments · Fixed by #1918 or #1919
Closed

Should I expect completion of nested command's flags to work? #1916

dearchap opened this issue May 25, 2024 Discussed in #1915 · 31 comments · Fixed by #1918 or #1919

Comments

@dearchap
Copy link
Contributor

Discussed in #1915

Originally posted by ldelossa May 25, 2024
Hello,

I have a root command called 'cmd'.
Now I have a sub command hanging off this called "sub-command".
The cli.Command for "sub-command" defines 2 flags "--a, --b".
All commands have the "EnableShellCompletion" boolean set to true.

I'm using zsh. Should I expect to see completion for cmd subcommand --<Tab>.
My assumption was to see completion for --a, --b but instead the only completion I get is for --h, -help essentially.

@dearchap
Copy link
Contributor Author

@ldelossa what release are you using ?

@ldelossa
Copy link

Hey there @dearchap

module github.com/ldelossa/cmds

go 1.22.2

require (
	github.com/urfave/cli/v3 v3.0.0-alpha9
	gopkg.in/yaml.v3 v3.0.1
)

So v3.

You can see my code here: https://github.com/ldelossa/cmds/blob/main/main.go#L129

Tho its a bit funky, since I'm generating a command graph based off a yaml config.

@dearchap
Copy link
Contributor Author

@ldelossa Its okay to generate the cmd graph from a yaml. I dont see any real issues with your implementation. It should work. You sourced the zsh completion script for this app correct ? Everytime you hit on the terminal for completion(for this app) the app will be run with the --generate-shell-completion arg so that the app can output the cmd/flags as needed. Have you tried putting this through a debugger to see whats going on ?

@ldelossa
Copy link

ldelossa commented May 27, 2024

@dearchap Here is a screencap of the issue, as you can see, I can not get tab completion for any flag hanging off the sub command 'automation'

2024-05-27T11.21.00-04.00.mp4

I'm not sure how I can debug the simulation of hitting tab at a particular location in the desired command.

For example, I want my debugger to stop at the code that generates the shell completion, but only after I have the string cmds automation rocky-linux -- as the prefix to completion. Does the --generate-shell-completion command take a string prefix?

Here is the output from just running the flag:

cmds --generate-shell-completion
extract-scripts:extracts all embedded scripts to XDG_CONFIG_DIR/cmds/scripts/
kernel:Various commands to assist with Linux Kernel development
ker:Various commands to assist with Linux Kernel development
rsync:Various Rsync helpers
rs:Various Rsync helpers
debug:Various helpers related to debugging software
automation:Scripts for automating server setup.
git:Various git helpers
g:Various git helpers
k8s:Various Kubernetes helpers
k:Various Kubernetes helpers
help:Shows a list of commands or help for one command
h:Shows a list of commands or help for one command

By the way, this is only a problem with flags, completion of arbitrarily nested sub-command names work fine.

@ldelossa
Copy link

@dearchap

Are you sure I should be seeing completion for flags?

~/go/cmds main*
01:38:11PM 🖳  ./cmds automation rocky-linux --generate-shell-completion

~/go/cmds main*
01:38:26PM 🖳  ./cmds automation --generate-shell-completion  
rocky-linux:Setup a Rocky Linux instance for development

~/go/cmds main*
01:38:32PM 🖳  ./cmds automation rocky-linux                            
NAME:
   cmds automation rocky-linux - Setup a Rocky Linux instance for development

USAGE:
   cmds automation rocky-linux [arguments...]

DESCRIPTION:
   Connects to a remote Rocky Linux instance and applies an automated setup.
   Once the setup is finished the instance is capable of compiling and running a custom Linux kernel.
   The automated setup process will interactively prompt for a password for the provided user.


OPTIONS:
   --remote value, -r value  The remote host
   --user value, -u value    The non-root user created for general usage
   --help, -h                show help (default: false)
2024/05/30 13:38:49 Required flags "remote, user" not set

I think the above demonstrates clearly that no flags are being generated for completion.

I debugged it a bit, but I never see the -- characters being submitted for any completion func to evaluate flags.

@ldelossa
Copy link

@dearchap

Here is a repo with a very lean reproduction: https://github.com/ldelossa/urfacecli_test

Just

go build -o main .
source ./main_autocomplete

And try to perform autocompletion for the flag hanging off subcommand in a ZSH instance. It does not work.

@dearchap
Copy link
Contributor Author

dearchap commented May 30, 2024

@ldelossa I tried out your lean code and it works for me. Are you in a zsh first of all ? also how are you invoking the program main or ./main . For me main is throwing some vague output whereas ./main works just fine with tab completion

% echo $ZSH_NAME
zsh
% echo $ZSH_VERSION
5.8

@ldelossa
Copy link

Very interesting!

Okay thank you for confirming this is an issue locally.

I always invoke with ./main.

But now, let me enter a zsh with absolute minimum config and see if it works.

@ldelossa
Copy link

Okay @dearchap

So I am on zsh 5.9 and this does not work. Here is an example on my system:

2024-05-30T15.44.59-04.00.mp4

And here is the script:

03:44:53PM 🖳  zsh -f
fedora% echo $ZSH_VERSION 
5.9
fedora% autoload -Uz compinit
fedora% compinit
fedora% source ./main_autocomplete
fedora% 
fedora% 
fedora% ./main command subcommand 
NAME:
   cli command subcommand

USAGE:
   cli command subcommand [command [command options]] [arguments...]

COMMANDS:
   help, h  Shows a list of commands or help for one command

OPTIONS:
   --flag value, -f value  flag
   --help, -h              show help (default: false)
fedora% ./main command subcommand --generate-shell-completion
help:Shows a list of commands or help for one command
h:Shows a list of commands or help for one command
fedora% 

@ldelossa
Copy link

Tried the same thing in a zsh 5.8 container:

03:50:38PM 🖳  docker run -v $(pwd):/src -it --rm zshusers/zsh:5.8
6befea125665# cd /src 
6befea125665# ls -la
total 4444
drwxr-xr-x. 1 1000 1000      88 May 30 18:13 .
drwxr-xr-x. 1 root root     158 May 30 19:50 ..
drwxr-xr-x. 1 1000 1000     196 May 30 18:14 .git
-rw-r--r--. 1 1000 1000     154 May 30 18:05 go.mod
-rw-r--r--. 1 1000 1000    1078 May 30 18:05 go.sum
-rwxr-xr-x. 1 1000 1000 4528215 May 30 18:12 main
-rw-r--r--. 1 1000 1000     662 May 30 18:12 main.go
-rw-r--r--. 1 1000 1000     750 May 30 18:11 main_autocomplete
6befea125665# autoload -Uz compinit
6befea125665# compinit
6befea125665# source ./main_autocomplete 
6befea125665# ./main command subcommand --
NAME:
   cli command subcommand

USAGE:
   cli command subcommand [command [command options]] [arguments...]

COMMANDS:
   help, h  Shows a list of commands or help for one command

OPTIONS:
   --flag value, -f value  flag
   --help, -h              show help (default: false)
6befea125665# ./main command subcommand --generate-shell-completion
help
h
6befea125665# 

@dearchap
Copy link
Contributor Author

Are you hitting TAB after the command and you see the subcommand ? If you hit the TAB after subcommand you should see h for help. The generate-shell-completion arg is automatically passed to the program when TAB is hit. You shouldn't need to type in --generate-shell-completion manually unless you are testing the autocomplete behavior . So what exactly is the problem here ? I'm confused

@ldelossa
Copy link

ldelossa commented May 30, 2024

@dearchap

I'm trying to get completion for the flag hanging off 'subcommand'.

For instance if I do this:

./main command subcommand -<TAB>

I expect to see completion suggestions like this:

-f --flag

That was the original question created in the discussion topic.

@dearchap
Copy link
Contributor Author

@ldelossa Ok I understand now. Yes looks like there is an issue. Let me see what I can do

@dearchap
Copy link
Contributor Author

@ldelossa Can you do export URFAVE_CLI_TRACING="on" and then try hitting the TAB during subcommand .?

@ldelossa
Copy link

No problem @dearchap

The output was very large so put it in a pastebin: https://pastebin.com/Y84QDy2h

@dearchap
Copy link
Contributor Author

@ldelossa Are you typing "--" at the end ? Can you try "-" and then hit the tab ?

@ldelossa
Copy link

@dearchap no problem, here's latest: https://pastebin.com/jtEf28Xt

@ldelossa
Copy link

We seem to be going into printCommandSuggestions instead of printFlagSuggestions when we are in the DefaultCompleteWithFlags

@dearchap
Copy link
Contributor Author

@ldelossa Yes you're right. The code doesnt make sense. Something isnt right. v2 has the correct behaviour, we seem to have broken it in v3. I will put in a fix. Thanks for raising this issue

@ldelossa
Copy link

No problem, looking forward to seeing it fixed :-D. Thanks for working with me on explaining this situation.

@ldelossa
Copy link

@dearchap

If i comment out:

		if cmd != nil && cmd.flagSet != nil && cmd.parent != nil {
			args = cmd.Args().Slice()
			tracef("running default complete with flags[%v] on command %[1]q", args, cmd.Name)
		} else {
			tracef("running default complete with os.Args flags")
		}

Things begin to work. Maybe this is a bad heuristic, since having defined flags does not necessary mean cmd.Args().Slice() returns anything. IIUC cmd.Args() returns explicitly defined arguments on the command structure.

@ldelossa
Copy link

ldelossa commented Jun 1, 2024

@dearchap

Unfortunately, your fix does not fix the issue.

Like I mentioned above, the replacement of 'args' here:

	if cmd != nil && cmd.flagSet != nil && cmd.parent != nil {
		args = cmd.Args().Slice()
		tracef("running default complete with flags[%v] on command %[2]q", args, cmd.Name)
	} else {
		tracef("running default complete with os.Args flags[%v]", args)
	}

Is breaking things.

As soon as I do this:

func DefaultCompleteWithFlags(ctx context.Context, cmd *Command) {
	args := os.Args
	if cmd != nil && cmd.flagSet != nil && cmd.parent != nil {
		// args = cmd.Args().Slice()
		tracef("running default complete with flags[%v] on command %[2]q", args, cmd.Name)
	} else {
		tracef("running default complete with os.Args flags[%v]", args)
	}
	argsLen := len(args)
	lastArg := ""
	// parent command will have --generate-shell-completion so we need
	// to account for that
	if argsLen > 1 {
		lastArg = args[argsLen-2]
	} else if argsLen > 0 {
		lastArg = args[argsLen-1]
	}

	if strings.HasPrefix(lastArg, "-") {
		tracef("printing flag suggestion for flag[%v] on command %[1]q", lastArg, cmd.Name)
		printFlagSuggestions(lastArg, cmd.Flags, cmd.Root().Writer)
		return
	}

	if cmd != nil {
		tracef("printing command suggestions on command %[1]q", cmd.Name)
		printCommandSuggestions(cmd.Commands, cmd.Root().Writer)
		return
	}
}

Everything works.

2024-06-01T15.58.15-04.00.mp4

@dearchap
Copy link
Contributor Author

dearchap commented Jun 1, 2024

@ldelossa did you use the latest fix I just merged ?

@ldelossa
Copy link

ldelossa commented Jun 1, 2024

Yes, dont you see the code present in the video I just sent? That is latest.

@dearchap
Copy link
Contributor Author

dearchap commented Jun 2, 2024

@ldelossa sorry yes I was reading the comments in my email and didnt watch the video. Let me dig into this a bit more.

@dearchap dearchap reopened this Jun 2, 2024
@dearchap
Copy link
Contributor Author

dearchap commented Jun 2, 2024

@ldelossa The problem is much more complex than I first thought. Can you try with latest code(without your changes) and try just "-" and TAB instead of "--" and TAB ?

@ldelossa
Copy link

ldelossa commented Jun 2, 2024

@dearchap yup, I tried both, both do not complete.

@dearchap
Copy link
Contributor Author

dearchap commented Jun 2, 2024

@ldelossa if you comment out the args it might work with flags but will fail with subcommands.

main command <TAB>

@ldelossa
Copy link

ldelossa commented Jun 2, 2024

Not for me. Everything works when i comment that out. This, i don't know about positional argument competition. I don't use that only flags

@dearchap
Copy link
Contributor Author

dearchap commented Jun 2, 2024

@ldelossa can you try fix from latest PR ? Kind of kludgy but works

@ldelossa
Copy link

ldelossa commented Jun 3, 2024

hey @dearchap running out of cycles to keep testing this unfortunately, I provided the test example which should determine if your fixes work or not. I would just ensure to check for all tab completion "-", "--", arguments and subcommands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants