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.

Sign up for GitHub

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

Jump to bottom

Declare --stdin with FILE argument for better help text #4226

Merged
merged 1 commit into from
Apr 1, 2017
Merged

Declare --stdin with FILE argument for better help text #4226

merged 1 commit into from
Apr 1, 2017

Conversation

jonas054
Copy link
Collaborator

Before this change it is not apparent from the help text that -s/--stdin takes a file name argument.

@jonas054 jonas054 force-pushed the better_stdin_help branch from 2d94a1c to 8ba169e Compare March 31, 2017 19:20
bbatsov
bbatsov reviewed Apr 1, 2017
View reviewed changes
lib/rubocop/options.rb Outdated
if @options[:stdin] && !args.one?
raise ArgumentError, '-s/--stdin requires exactly one path.'
if @options[:stdin]
if args.any?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given the error message shouldn't we check if args has a size of 1?

Copy link
Collaborator Author

@jonas054 jonas054 Apr 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

With this change the file name given to --stdin is now an argument to the option, so it doesn't go into args. I can add a comment clarifying this. Or change the message to "No other files can be given when --stdin is used"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or you can do both. :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about the wording of the error message, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the end I couldn't come up with a message that better than the one we've got, so I only updated the comments.

@jonas054
Declare --stdin with FILE argument for better help text
4b67947 
Before this change it is not apparent from the help text
that -s/--stdin takes a file name argument.
@jonas054 jonas054 force-pushed the better_stdin_help branch from 8ba169e to 4b67947 Compare April 1, 2017 11:48
@bbatsov bbatsov merged commit 47f8671 into rubocop:master Apr 1, 2017
@savef
Copy link
Contributor

savef commented Apr 5, 2017 •
edited
Loading

Hi @jonas054, this has broken an Atom plugin that I use, linter-rubocop. It tries to use a command like the following, which used to work in v0.48.0, but now fails:
bin/rubocop --stdin --display-style-guide /path/to/file
Due to triggering the error:
-s/--stdin requires exactly one path.

Before I make an issue... I'm just wondering if this new stricter behaviour is intended, if it is I'll make the issue/PR in their repo instead.

@jonas054
Copy link
Collaborator Author

jonas054 commented Apr 5, 2017

@savef The new stricter behavior is sort of a byproduct of the better description from --help. But we should stick with it IMO. Ask linter-rubocop to call bin/rubocop --display-style-guide --stdin /path/to/file

@jonas054 jonas054 deleted the better_stdin_help branch April 5, 2017 14:19
@savef
Copy link
Contributor

savef commented Apr 5, 2017 •
edited
Loading

Never mind, it looks like a newer version of the plugin happened to do a re-write (CS -> ES6) and proactively fixed my issue as part of that. 😄

I had decided the new RuboCop behaviour was probably the correct one anyway.

Edit: Thanks, @jonas054!

@jonas054
Copy link
Collaborator Author

jonas054 commented Apr 5, 2017

OK! 😄

yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 5, 2017
@yash-nisar
RuboCopBear: Fix CI breakage
dcda805 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. Also, the tests
were enhanced that involved the bear to be run on files with the `.rb`
(ruby) extension because initially the bear was run on the generated
temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 5, 2017
@yash-nisar
RuboCopBear: Fix CI breakage
8794304 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. Also, the tests
were enhanced that involved the bear to be run on files with the `.rb`
(ruby) extension because initially the bear was run on the generated
temporary files that was the cause of error.

Fixes coala#1548
@yash-nisar yash-nisar mentioned this pull request Jul 5, 2017
Merged
2 tasks
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 5, 2017
@yash-nisar
RuboCopBear: Fix CI breakage
97d1d70 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. Also, the tests
were enhanced that involved the bear to be run on files with the `.rb`
(ruby) extension because initially the bear was run on the generated
temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 6, 2017
@yash-nisar
RuboCopBear: Fix CI breakage
0414740 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 6, 2017
@yash-nisar
RuboCopBear: Fix CI breakage for version > 0.47.1
e1139dc 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 6, 2017
@yash-nisar
RuboCopBear: Fix CI breakage for version > 0.47.1
2359913 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 6, 2017
@yash-nisar
RuboCopBear: Fix CI breakage for version > 0.47.1
51507c5 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
yash-nisar added a commit to yash-nisar/coala-bears that referenced this pull request Jul 6, 2017
@yash-nisar
RuboCopBear: Fix CI breakage for version > 0.47.1
d35b120 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
gosom pushed a commit to gosom/coala-bears that referenced this pull request Jul 15, 2017
@yash-nisar
RuboCopBear: Fix CI breakage for version > 0.47.1
866c3f1 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
@gerrywastaken gerrywastaken mentioned this pull request Jul 22, 2017
Closed
This was referenced Aug 4, 2017
Closed
Closed
Merged
Merged
umeshksingla pushed a commit to umeshksingla/coala-bears that referenced this pull request Sep 2, 2017
@yash-nisar @umeshksingla
RuboCopBear: Fix CI breakage for version > 0.47.1
4b0e427 
rubocop/rubocop#4226 indicates that `--stdin`
takes filename as an argument which was incorporated. A part of the
fixing was to improve the tests that involved the bear to be run on
files with the `.rb`(ruby) extension because initially the bear was
run on the generated temporary files that was the cause of error.

Fixes coala#1548
@dependabot dependabot bot mentioned this pull request Mar 12, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsov bbatsov bbatsov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jonas054 @savef @bbatsov