Confusion around how --quiet and --fix should behave together

[platinumazure]

See #8353 for background.

I think the best way to fix this is to allow users the option to go either way and to be very explicit about how the options work.

Here's one proposal along those lines:

• Rename --quiet to --no-report-warnings (supporting --quiet as an alias and preserving its behavior), to avoid reporting warnings but possibly fix them
• Add option --no-fix-warnings to support not running autofix on warnings but possibly report them (no effect if --fix not specified)
• Users can use both to completely ignore warnings altogether

With this proposal, our CLI options become more clear and more flexible with regard to whether warnings are reported/logged (--quiet/--no-report-warnings) and whether warnings are fixed (--no-fix-warnings).

I'll champion.

[soda0289]

Maybe we could add a third option to not report and not fix, something like --ignore-warnings.

[not-an-aardvark]

Could that be achieved by using --no-fix-warnings and --no-report-warnings at the same time?

[platinumazure]

I agree with @not-an-aardvark, we probably wouldn't need a third option.

…

On Jun 3, 2017 12:51 PM, "Teddy Katz" ***@***.***> wrote: Could that be achieved by using --no-fix-warnings and --no-report-warnings at the same time? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#8675 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARWepBmyrYJuhJJV69JMpHtA8Ox9PCKks5sAZ0lgaJpZM4NtCGA> .

[soda0289]

It could it would just alias those two options. I understand that adding too many options to the CLI would be a problem. I'm fine with just having the two options.

[mysticatea]

In my first impression, it's surprisable that warnings still fix code with --no-report-warnings. I think better that people cannot choose "not reported but fixed".

How about?

Option	Reported	Fixed
(default)	Yes	Yes
--ignore-warnings=fix	Yes	No
--ignore-warnings=full (alias --quiet)	No	No

[platinumazure]

@mysticatea Yes, I think you're right, that behavior is unexpected (and was the root of the original issue). I like the direction you're going, but maybe we could bikeshed the name a little bit?

How about this?

• --warnings=report-and-fix (default)
• --warnings=report-only (no fix, but report warnings)
• --warnings=none (don't report or fix warnings, alias --quiet)

[not-an-aardvark]

So are we proposing that the behavior of --quiet should change to not fix warnings? At the moment, warnings are fixed but are not reported with --quiet.

[platinumazure]

I would strongly prefer that `--quiet` not fix warnings, as it seems unintuitive, unexpected, and dangerous. Maybe `--quiet` should be deprecated (and removed in next major) in favor of a more explicit option from one of the proposals here. Or, if we think `--quiet` fixing warnings might be a bug (I certainly consider it one), then it could be done as a semver-patch or semver-minor change. I would be okay with that too. In that case, I think there is still room for a new option for "report but don't fix warnings".

…

On Jun 4, 2017 22:04, "Teddy Katz" ***@***.***> wrote: So are we proposing that the behavior of --quiet should change to not fix warnings? At the moment, warnings are fixed but are not reported with --quiet. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#8675 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARWelI5w1rbKkftTYPZgHbzrNFLqC1Aks5sA3BXgaJpZM4NtCGA> .

[not-an-aardvark]

I wonder if it would be better to just change --quiet to not fix warnings, and not introduce any other options. Then --warnings=report-and-fix would just be the default with --fix, and --warnings=report-only could be simulated by running with --fix --quiet once, and then again without --fix.

I'm not sure I like that the --warnings option would be meaningless unless --fix is also used.

[mysticatea]

Just arranged.
Granted, looks good to me if we change that ※ as a bug.

Option	Error reported	Error fixed	Warning reported	Warning fixed
(default)	✔		✔
--quiet	✔
--fix	✔	✔	✔	✔
--fix --quiet	✔	✔		※

※ Currently, warnings are fixed but not reported.

[platinumazure]

I am 100% behind @mysticatea's latest proposal (just fix --quiet). If a few other people (especially TSC) get behind it, I'll change the top post and reclassify this as a bug.

I'm not super attached to adding a "report but don't fix warnings" option in this PR. The thing I'm really championing here is making it so people can completely silence warnings (both report and fix), which --quiet should probably do.

[platinumazure]

Created #8699 for if we want to treat this as a bugfix. If we decide this should be treated as a feature or enhancement, I can close that PR.

[alberto]

ping @gyandeeps since you disagreed with changing --quiet behaviour

Another possibility would be to add options to --fix: all (default), error, warning (not sure this one makes a lot of sense)

[platinumazure]

If @gyandeeps still disagrees with changing --quiet behavior, should this perhaps go on TSC agenda?

Now that #8730 is merged, I'm willing to write a PR using that functionality to avoid autofixing warnings when --quiet is passed. I might hold off a day or two to see if TSC might want this to go a different direction.