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

[report] Honour -o/-e/-n from cmdline over preset #3195

Merged
merged 1 commit into from
Apr 25, 2023
Merged

[report] Honour -o/-e/-n from cmdline over preset #3195

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions sos/component.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,21 @@ def apply_options_from_cmdline(self, opts):
# set all values back to their normal default
codict = cmdopts.dict(preset_filter=False)
for opt, val in codict.items():
if opt not in cmdopts.arg_defaults.keys():
if opt not in cmdopts.arg_defaults.keys() or val in [None, [], '']:
continue
if val not in [None, [], ''] and val != opts.arg_defaults[opt]:
# A plugin that is [enabled|disabled|only] in cmdopts must
# overwrite these three options of itself in opts - reset it first
if opt in ["enable_plugins", "skip_plugins", "only_plugins"]:
for oopt in ["enable_plugins", "skip_plugins", "only_plugins"]:
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually need the loop here? We have the previous conditional for checking that we're operating on one of those keys, so I don't think we need to loop through each of those keys each time we meet the conditional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to evaluate pairs of options+values, if present. Like "-e logs in cmdopts AND -n logs in preset". So for each cmdopts that alters plugin enablement, we must check if it is not in a conflict of a different option for the same plugin.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok - that tracks. Thanks for clearing that up for me.

common = set(val) & set(getattr(opts, oopt))
# common has all plugins that are in this combination of
# "[-e|-o|-n] plug" of cmdopts & "[-e|-o|-n] plug" of opts
# so remove those plugins from this [-e|-o|-n] opts
if common:
setattr(opts, oopt, [x for x in getattr(opts, oopt)
if x not in common])

if val != opts.arg_defaults[opt]:
setattr(opts, opt, val)

return opts
Expand Down