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

-n option doesn't disable colors #4168

Closed
sadovnik opened this issue Mar 25, 2017 · 6 comments
Closed

-n option doesn't disable colors #4168

sadovnik opened this issue Mar 25, 2017 · 6 comments
Labels

Comments

@sadovnik
Copy link
Contributor

sadovnik commented Mar 25, 2017

Expected behavior

rubocop -n runs Rubocop without coloring the output.

Actual behavior

Rubocop runs with colors.

Steps to reproduce the problem

rubocop -n

RuboCop version

0.47.1 (using Parser 2.4.0.0, running on ruby 2.3.1 x86_64-darwin15)

The cause

Here's what I found out. There was only --no-color option, without optional [no-] part. When --[no-]color was introduced, the shorthand version of this flag (-n) wasn't taken into account.
a5ce6df#diff-aabedc1d8225971d14fe2ceecc1be1e1

OptionParser just don't knows that -n is a negative flag:

require 'optparse'

option_parser = OptionParser.new do |cli|
  cli.on('-n', '--[no-]color', 'Force color output on or off.') do |color|
    p color
  end
end

option_parser.parse(['--color'])
option_parser.parse(['--no-color'])
option_parser.parse(['-n'])
true
false
true

I suggest to drop this flag.

What do you think?

@rrosenblum
Copy link
Contributor

It looks like you found the reason for this issue already. It seems like the only options are to split this out so there is a --color option and a -n, --no-color option, or split it out so that we have a --[no-]color option and a -n option that sets no color. I am not sure which would be preferred or if anyone has another idea how to fix this. What are your thought @bbatsov?

@rrosenblum
Copy link
Contributor

I guess another other option could be to remove the -n flag since it seems like it has been broken for over a year, and as far as I know, this is the first that anyone has mentioned it.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 28, 2017

I guess another other option could be to remove the -n flag since it seems like it has been broken for over a year, and as far as I know, this is the first that anyone has mentioned it.

Yeah, probably this functionality doesn't warrant having short option names at all.

@bbatsov bbatsov added the bug label Mar 28, 2017
@sadovnik
Copy link
Contributor Author

@bbatsov may I submit PR that removes the short option?

@rrosenblum
Copy link
Contributor

@sadovnik PRs are always welcome. I don't think anyone has started this work, so I say go for it.

@Drenmi
Copy link
Collaborator

Drenmi commented Mar 30, 2017

I guess another other option could be to remove the -n flag since it seems like it has been broken for over a year, and as far as I know, this is the first that anyone has mentioned it.

Yeah, probably this functionality doesn't warrant having short option names at all.

Agree. Also -n is not the most intuitive option for disabling color. 🙂

sadovnik added a commit to sadovnik/rubocop that referenced this issue Mar 31, 2017
This option was broken in a5ce6d and the community decided to remove it. Here's why:
* Nobody used it. It has been broken for over a year before it was noticed it's broken
* `-n` is not the most intuitive option for disabling color
* There is no plain way to fix it
sanssecours added a commit to sanssecours/ruby.tmbundle that referenced this issue Apr 19, 2017
The latest version of `rubocop` does not support the flag `-n` anymore
([1]).

[1]: rubocop/rubocop#4168
sanssecours added a commit to sanssecours/ruby.tmbundle that referenced this issue Apr 20, 2017
The latest version of `rubocop` does not support the flag `-n` anymore
([1]).

[1]: rubocop/rubocop#4168
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants