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

Return non-zero exit code on invalid commands #4135

Merged
merged 2 commits into from
May 11, 2021

Conversation

neoaggelos
Copy link
Contributor

Summary

Closes #4091

Replaces #4129, thanks @htdvisser for suggesting a much more elegant solution.

Changes

  • If Run and RunE are empty, no sub-command has been matched, so return a non-zero exit status code.

Testing

Locally

Regressions

There should not be any

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@github-actions github-actions bot added the ui/cli This is related to ttn-lw-cli label May 5, 2021
@@ -59,6 +59,9 @@ var (
SilenceUsage: true,
Short: "The Things Network Command-line Interface",
PersistentPreRunE: preRun(checkAuth, refreshToken, requireAuth),
RunE: func(cmd *cobra.Command, args []string) error {
return nil
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@htdvisser Added this otherwise running ttn-lw-cli --help returns a non-zero exit status code as well (which makes the CI complain).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's the right solution, because now we would have inconsistent error codes when requesting help on different commands.

How about putting this in main.go:

help := commands.Root.HelpFunc()
commands.Root.SetHelpFunc(func(cmd *cobra.Command, a []string) {
	help(cmd, a)
	os.Exit(0)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work as expected, since the help function will be called for invalid commands as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps something slightly more elaborate, like:

help := commands.Root.HelpFunc()
commands.Root.SetHelpFunc(func(cmd *cobra.Command, a []string) {
	help(cmd, a)
	for _, arg ;= range a {
	    if a == "--help" {
	         os.Exit(0)
	    }
        }
})

But this is also getting a bit messy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay so maybe we should just accept an exit code 2 in CI, or use a different command for testing. Perhaps have CI call ttn-lw-* config instead?

@neoaggelos neoaggelos requested a review from htdvisser May 5, 2021 15:55
Copy link
Contributor

@htdvisser htdvisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment from last week.

@neoaggelos neoaggelos force-pushed the issue/4091-error-on-invalid-subcommand branch from 80af8d7 to 6f2aa88 Compare May 10, 2021 10:27
@neoaggelos neoaggelos force-pushed the issue/4091-error-on-invalid-subcommand branch from 6f2aa88 to f619d8d Compare May 10, 2021 10:30
@github-actions github-actions bot added the tooling Development tooling label May 10, 2021
@neoaggelos neoaggelos merged commit 03fc4cd into v3.13 May 11, 2021
@neoaggelos neoaggelos deleted the issue/4091-error-on-invalid-subcommand branch May 11, 2021 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Development tooling ui/cli This is related to ttn-lw-cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ttn-lw-cli exits 0 with invalid subcommand
2 participants