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

PICARD-2271: Add Options page to allow removing unused settings from INI file. #1893

Merged
merged 17 commits into from
Sep 3, 2021

Conversation

rdswift
Copy link
Collaborator

@rdswift rdswift commented Aug 31, 2021

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

Over the course of trying out plugins and such, the configuration INI file can become bloated with orphaned settings.

Solution

Provide a new Options page (called "Maintenance") that lists all orphaned settings found in the INI file, and allows the user to mark the settings for deletion.

This allows you to remove unused option settings from the configuration file.

Settings that are found in the configuration file that do not appear on any option settings page will be listed. If the configuration file does not contain any unused option settings, then the list will be empty.

Note that unused option settings could come from plugins that have been uninstalled, so care must be taken to not remove settings that you may want to use later when the plugin is reinstalled. Options belonging to plugins that are installed, including those that are currently disabled, will not be listed for possible removal.

To remove one or more settings, select the settings to remove by checking the box next to the setting, and then enable the removal by checking the Remove selected options box. When you choose Make It So! to save your option settings, the selected items will be removed.

Action

If this change is accepted, the documentation will need to be updated.

Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

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

I like the feature.

picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
- Move `import` to top of module.
- Refactor to remove `_add_options_from_page()` method.
- Use standard QSettings methods to get `file_options` set.
- Add comment explaining the calculation for `row_height`.
@zas zas requested a review from phw September 1, 2021 19:04
@rdswift rdswift mentioned this pull request Sep 1, 2021
34 tasks
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
- Use more descriptive variable name `option_name`.
- Set value as PlainText.
- Store option keys as `UserRole` data.
- Remove redundant (default) options to `QMessageBox`.
- Use `repr()` to return option values.
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
zas
zas previously approved these changes Sep 2, 2021
Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

Looks good, good work. Amazing what stuff I found in my config file :)

I'd just move it under the Advanced section, what do you think?

@rdswift
Copy link
Collaborator Author

rdswift commented Sep 3, 2021

Amazing what stuff I found in my config file :)

Me too. It's amazing all the stuff that accumulates in your test configuration file during development. 😜

I'd just move it under the Advanced section, what do you think?

Perfect! I'll make the change right away.

phw
phw previously approved these changes Sep 3, 2021
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
picard/ui/options/maintenance.py Outdated Show resolved Hide resolved
zas
zas previously approved these changes Sep 3, 2021
Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

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

LGTM

zas
zas previously approved these changes Sep 3, 2021
Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

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

LGTM

@zas zas merged commit f14e415 into metabrainz:master Sep 3, 2021
@rdswift rdswift deleted the clean_unused_config_options branch September 3, 2021 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants