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

shell shows error and usage when given extra parameters #9095

Merged

Conversation

stop-start
Copy link

Fixes #8593

When try to launch the shell with arguments the shell won't launch, instead will show an error (with the unknown args) as well as the usage.

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)

@ghost ghost added the proposed label Nov 13, 2017
@@ -108,6 +109,13 @@ Examples:
}
fs.Parse(os.Args[1:])

argsNotParsed := os.Args[len(os.Args)-fs.NArg():]
if len(argsNotParsed) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This block should use flag.Args() instead.

https://golang.org/pkg/flag/#Arg

Args returns the non-flag command-line arguments.

@mark-rushakoff
Copy link
Contributor

@stop-start would you mind adding a line like

- [#9065](https://github.com/influxdata/influxdb/pull/9065): Refuse extra arguments to influx CLI

to the 1.5.0 / Bugfixes section of CHANGELOG.md?

@stop-start
Copy link
Author

@mark-rushakoff Done

@mark-rushakoff mark-rushakoff merged commit a1329cc into influxdata:master Nov 13, 2017
@ghost ghost removed the proposed label Nov 13, 2017
@mark-rushakoff
Copy link
Contributor

Thanks @stop-start!

@stop-start
Copy link
Author

Thanks @mark-rushakoff, it's my second pull request but first looked at and approved!

@Luit
Copy link

Luit commented Nov 29, 2017

The PR number in CHANGELOG.md is the wrong one, though. 9065 instead of 9095.

@mark-rushakoff
Copy link
Contributor

Good catch @Luit, that was a typo on my part. Care to open a small PR to fix it?

@mark-rushakoff
Copy link
Contributor

I opened a PR to fix it in #9182.

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

Successfully merging this pull request may close these issues.

None yet

3 participants