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

Ability to add nested commands through cobra add #266

Closed
ianwalter opened this issue Apr 7, 2016 · 20 comments
Closed

Ability to add nested commands through cobra add #266

ianwalter opened this issue Apr 7, 2016 · 20 comments
Labels
area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior

Comments

@ianwalter
Copy link
Contributor

I would love to use cobra add to create nested commands, for example:

cobra add db migrate

Cobra would then see that there are multiple arguments, create a directory called db if it doesn't exist and a command file inside of it called migrate.go. It would also create another file either inside of db or at the same directory level called db.go or root.go that will simply create a dbCommand so that nested commands can be added to it. I can submit a PR for this after my existing PR is resolved. Thanks.

@spf13
Copy link
Owner

spf13 commented Apr 27, 2016

@ianwalter I love the idea, please contribute this.

If I can, I'll provide my ideas on what this feature should do/work like.

I think that the syntax should be cobra add db/migrate.
This would create db.go (if needed), db/ (if needed) and db/migrate.go.
It would also add the appropriate AddCommand(addCmd) calls to support the tree structure.

Currently the add command does support the --parent flag which does operate in a similar way, but requires more steps and doesn't put it in a subdirectory.

@ianwalter
Copy link
Contributor Author

That sounds good to me. I will work on this. I must have missed --parent. Should I leave it intact? It seems to me that it's functionality would be completely covered by the proposed solution.

@spf13
Copy link
Owner

spf13 commented Apr 27, 2016

Lets leave it for now. It works slightly differently and supports a few use
cases this doesn't.
On Wed, Apr 27, 2016 at 2:30 PM Ian Walter notifications@github.com wrote:

That sounds good to me. I will work on this. I must have missed --parent.
Should I leave it intact? It seems to me that it's functionality would be
completely covered by the proposed solution.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#266 (comment)

@waitingkuo
Copy link

Currently cobra add db/migrate does generate the subdirectory. But the behavior is strange. It generates a var db/migrate = &cobra.Command{}. Should we warn when the command contains "/" first?

@spf13
Copy link
Owner

spf13 commented Sep 4, 2016

I don't think it should warn. I think it should just do the right thing.
Seems like the only bug is how the variable name is created. Should we just
strip the "/"?
On Sun, Sep 4, 2016 at 6:22 AM Wei-Ting Kuo notifications@github.com
wrote:

Currently cobra add db/migrate does generate the subdirectory. But the
behavior is strange. It generates a var db/migrate = &cobra.Command{}.
Should we warn when the command contains "/" first?


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#266 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAKlZPRgOSRjWlxhioatEActsU7fjCAJks5qmpvdgaJpZM4ICUNR
.

@n10v n10v added area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior labels Apr 22, 2017
@kasisnu
Copy link

kasisnu commented Dec 28, 2018

Looks like this bug still exists. I'm happy to work on this.

@spf13
Copy link
Owner

spf13 commented Jan 1, 2019 via email

@Wulfheart
Copy link

@kasisnu what's the current status?

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@geniass
Copy link

geniass commented May 26, 2020

For me this doesn't create the subdirectory automatically.

Once I create the subdir, it creates a .go file inside it, but as discussed above the file has invalid syntax.

The next problem is that the subdir is considered a separate package (I think?)
Since the parent command, e.g. rootCmd is not exported, it can't be directly referenced.

One more problem is that since the command is now in a "sub-package", that sub-package needs to be explicitly imported somewhere (e.g. main) otherwise it won't even be compiled.

@spf13 Should we make the parent command public? Add a public function? Something else?

@geniass
Copy link

geniass commented May 26, 2020

@joshuabezaleel I think this issue is a duplicate right?
#1059

@joshuabezaleel
Copy link

@geniass Yes, I think these 2 issues convey the same problem statement.
I saw that there is a PR submitted already by @Wulfheart (?)

@Wulfheart
Copy link

Gonna reply later or in a few days.

@joshuabezaleel
Copy link

No worries. Will be happy to help and contribute if I can be of one @Wulfheart 🙂

@johnraz
Copy link

johnraz commented Apr 11, 2021

@joshuabezaleel did you find / build a solution to this?

Right now I managed to handle it by following the structure suggested here #641 (comment) but it makes the cobra cli kinda impossible to use with subcommands.

I'll attempt to summarise the situation with a simple example (and my very sparse Go knowledge):

We want to have a parent command exposing a child command sub.

The directory tree should look like:

├── cmd
│   ├── parent
│   │   ├── parent.go
│   │   └── sub.go
│   └── root.go
└── main.go
  • Using cobra add parent/sub (do we agree on the syntax?) should create the parent directory if it doesn't exist.
  • parent.go and sub.go should be part of the parent package

The parent package need to be somehow imported in order to be compiled.

Solution 1:

  • The rootCmd could be made public by renaming to RootCmd in order to be importable in the parent.go file.
  • The parent package should then be imported in the main.go file like this import _ "github.com/whatver/cmd/db" for the package to be picked-up by the compiler and added properly.

Solution 2:

  • The parentCmd could be made public by renaming to ParentCmd in order to be importable in the root.go file.
  • It could then be used in the init of the root.go file like this:
func init() {
	RootCmd.AddCommand(db.Cmd)
}
  • Due to the fact that the parent package is now imported in root.go, it is picked up by the compiler and added properly.

@spf13 , @Wulfheart any comments?
Does that sound like a reasonable approach and something that you'd still like to see added to the cli command?
Are there alternative approach?

Thanks for your time 😸

@rulisastra
Copy link

rulisastra commented Jan 20, 2022

Hi @johnraz

I successfully follow your Solution 2 but I am getting this error.

package github.com/whatver
        imports github.com/whatver/cmd
        imports github.com/whatver/cmd/parent
        imports github.com/whatver/cmd: import cycle not allowed

I don't know why. I have followed all your steps.
I get that it was get cycled that's why I try to delete the init() inside the parent package also, but it shows the same cycle error

@johnSchnake
Copy link
Collaborator

Since this is a cobra-cli issue I'm going to close; it can be opened again in the new repo: https://github.com/spf13/cobra-cli/issues/

@rulisastra without being able to see your code its hard to help with the import cycle. The files in cmd/parent should not refer to cmd package at all though.

Also worth noting: I think the decision to put commands into subdirectories instead of a single large directory has some pros/cons. A related ticket to boilerplate project structure has been opened as well spf13/cobra-cli#12 and may be a good place to discuss that.

@lcarva
Copy link
Contributor

lcarva commented Feb 20, 2023

I went ahead with the solution 2 proposed above. It seems like a natural way to organize sub-commands, especially if there are multiple levels.

Would you be open to having this added to the docs? I'm happy to contribute

@spf13
Copy link
Owner

spf13 commented Mar 7, 2023

Please contribute. Solution 2 seems like a viable approach.

@jbcpollak
Copy link

Using Solution too I am getting an import cycle because a sub-command is trying to import root.go so that it can access a value saved in a PersistentFlags() call. For example, in cmd/root.go:

var BaseURL string
...
func init() {
	rootCmd.PersistentFlags().StringVarP(&BaseURL, "server-url", "s", url, "Base URL")
}

then cmd/sub/foo/foo.go tries to import rootCmd to access BaseURL.

What's the best way to avoid that? Should flags be persisted in a different package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.