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

Subcommand Group | panic: Group id 'Group1' is not defined for subcommand 'app1 sub1' #1831

Closed
KINGSABRI opened this issue Oct 17, 2022 · 7 comments · Fixed by #1839
Closed

Comments

@KINGSABRI
Copy link

KINGSABRI commented Oct 17, 2022

Hi everyone
I'm trying to implement the subcommand group feature introduced in v[v1.6.0]. I followed the test code

/app1/cmd/root.go

func init() {
	cobra.EnableCaseInsensitive = true
	cobra.EnableCommandSorting = false

	rootCmd.CompletionOptions.DisableDefaultCmd = true 
	rootCmd.Flags().SortFlags = false
	rootCmd.PersistentFlags().SortFlags = false

	rootCmd.AddGroup(&cobra.Group{ID: "Group1", Title: "Group1 Subcommands:"})
}

/app1/cmd/sub1.go

var sub1Cmd = &cobra.Command{
	Use:     "sub1",
	GroupID: "Group1",
	Aliases: []string{"1"},
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("Hiii")
	},
}

func init() {
	rootCmd.AddCommand(sub1Cmd)
	sub1Cmd.Flags().SortFlags = false
	sub1Cmd.Flags().StringP("ipaddress", "i", "0.0.0.0", "IP address to listen to")
}

When I run the app I get

panic: Group id 'Group1' is not defined for subcommand 'app1 sub1'
goroutine 1 [running]:
github.com/spf13/cobra.(*Command).AddCommand(0xd024e0, {0xc00024fd08, 0x1, 0x1?})
        /home/USER/go/pkg/mod/github.com/spf13/[email protected]/command.go:1223 +0x2d4
app1/cmd.init.3()
        /home/USER/Code/app1/cmd/sub1Cmd.go:47 +0x54

Q\ is there any complex example that wrapup all cobra's features?

@KINGSABRI
Copy link
Author

Hi,
I'm not sure why the issue is being submitted twice.

Your advice regarding the issue is appreciated

Thanks

@andig
Copy link

andig commented Oct 17, 2022

Same here. It's not obvious where the Groups need be added in order to be specified on the child command?

/cc @aawsome @marckhouzam

@aawsome
Copy link
Contributor

aawsome commented Oct 18, 2022

The AddGroup command must be executed before the AddCommand(sub1Cmd) is executed.

As both are in the init() functions in your example, according to https://stackoverflow.com/questions/32829538/init-order-within-a-package they are executed in lexical order, i.e. init() of sub1.go before init() of root.go - which is the wrong order.

EDIT: I think you renamed your real subcommand which started with a letter lower than r in root.go into sub1.go - using sub1.go should actually work.

So, to solve your problem, either rename your files such that the file name of the go file defining the subcommand is lexical larger than the file name of the go file defining the root command.

Another alternative is to change init() in sub1.go into initSub1() and add a initSub1() at the end of the init() function in root.go.

@andig
Copy link

andig commented Oct 18, 2022

Thanks for the explanation. This is a bit unfortunate from usability perspective as it introduces coupling between otherwise independent parts (except for being aware that there is a rootCmd). It would be helpful if the groups could be declared right when the rootCmd is first assigned (didn't check if that's the case due to internet issues, sorry).

Update I would suggest to make the commandgroups or Groups public API.

@KINGSABRI
Copy link
Author

Thank @aawsome for your answers and the solutions. I would've never guessed it from AddGroup() documentation. I also didn't know about Go's lexical order during the compilation.

I think you renamed your real subcommand
True

The alternative solution works just fine for me

/app1/cmd/root.go

func init() {
	cobra.EnableCaseInsensitive = true
	cobra.EnableCommandSorting = false

	rootCmd.CompletionOptions.DisableDefaultCmd = true 
	rootCmd.Flags().SortFlags = false
	rootCmd.PersistentFlags().SortFlags = false

	rootCmd.AddGroup(&cobra.Group{ID: "Group1", Title: "Group1 Subcommands:"})

	initSub1()
	initSub2()
	initSub3()
}

/app1/cmd/sub1.go

var sub1Cmd = &cobra.Command{
	Use:     "sub1",
	GroupID: "Group1",
	Aliases: []string{"1"},
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("Hiii")
	},
}

func initSub1() {
	rootCmd.AddCommand(sub1Cmd)
	sub1Cmd.Flags().SortFlags = false
	sub1Cmd.Flags().StringP("ipaddress", "i", "0.0.0.0", "IP address to listen to")
}

I also agree with @andig regarding the inconvenience usability of this approach

Thank you all

@andig
Copy link

andig commented Oct 19, 2022

@KINGSABRI could you kindly keep this open for the usability aspect? Having to implement this like you just did is really bad.

@marckhouzam marckhouzam reopened this Oct 19, 2022
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Oct 23, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Oct 23, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Oct 24, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Oct 24, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Oct 24, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
marckhouzam added a commit that referenced this issue Oct 24, 2022
Fixes #1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
@andig
Copy link

andig commented Oct 24, 2022

Much appreciated, thank you!

jpmcb pushed a commit to jpmcb/cobra that referenced this issue Oct 24, 2022
Fixes spf13#1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>
jpmcb added a commit that referenced this issue Oct 24, 2022
Fixes #1831

By moving the check for help group existence to "ExecuteC()" we no
longer need groups to be added before AddCommand() is called.  This
provides more flexibility to developers and works better with the use
of "init()" for command creation.

Signed-off-by: Marc Khouzam <[email protected]>

Signed-off-by: Marc Khouzam <[email protected]>
Co-authored-by: Marc Khouzam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants