-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Allow disabling bad-option-value
and unrecognized-option
with --disable=all
#6691
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
Allow disabling bad-option-value
and unrecognized-option
with --disable=all
#6691
Conversation
β¦disable=all` Previously, due to the order in which options are parsed (config file, then command line), any unrecognized or bad options in the config file would emit messages unconditionally, even if suppressed on the CLI.
Pull Request Test Coverage Report for Build 2397585660
π - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could have a mechanism in place to signal that the configuration is parsed (boolean config_is_parsed ?) and use a stash directly in add_message
for any message if the configuration is not parsed yet.
I guess I didn't want to generalize it because the use case is so niche and we don't want to encourage or suggest using it for other messages (these are the only messages that it makes sense to emit before configuration is finished). There's also the matter of needing to stashing the location if we generalize it, which increases complexity (and might needlessly change the signature of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done getting this to work @jacobtylerwalls
Just one question:
Not sure if you thought about this but what about making stashed_bad_option_value_messages
a dictionary with keys reflecting the current module. We could use linter.current_name
to look up the correct name in the dictionary to put the message under in the callback action and then just iterate over the keys in emit_bad_option_value
(and reset current_name
there).
At some point I want to be able to parse multiple configuration files at once to create different config namespaces for sub-directories. So, just messages_from_config_file
might not be enough as we would need access to the different config file names.
Besides it would remove the need for L61-64 which is arguably a bit hacky. (The need for set_module
before add_message
is horrible anyway).
Another thing is that stashed_bad_option_value_messages
should probably be _stashed_bad_option_value_messages
to avoid any future deprecation issues.
Nice. Yeah I went that direction and paused as I wasn't sure of the use case, so that's helpful to hear it would be used! Will do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
Type of Changes
Description
Previously, due to the order in which options are parsed (config file, then command line), any unrecognized or bad options in the config file would emit messages unconditionally, even if suppressed on the CLI.
Closes #6565