badfiles: added config/cli options to automatically take action on error/warning

[tommyschnabel]

Description

Updated the badfiles plugin to support automatic actions taken on warning or error imports, to be used with check_on_import: yes.

Example usage:

badfiles:
    check_on_import: yes
    import_action_on_warning: continue
    import_action_on_error: skip

The above configuration will skip badfiles that error out, but ignore any warnings found.
The default for both options is ask, preserving current behavior.

To Do

• Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
• Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
• Tests. (Very much encouraged but not strictly required.)

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The logic for detecting warnings/errors by checking if the words 'warning' or 'error' appear in error_line.lower() is brittle (e.g. localization, different phrasing); consider passing structured severity information from the check instead of inferring it from the message text.
• The handle_import_action function hardcodes the allowed action strings in multiple places; consider centralizing the set of valid actions (e.g. constant or enum-like structure) to avoid drift and to make configuration validation clearer.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The logic for detecting warnings/errors by checking if the words 'warning' or 'error' appear in `error_line.lower()` is brittle (e.g. localization, different phrasing); consider passing structured severity information from the check instead of inferring it from the message text.
- The `handle_import_action` function hardcodes the allowed action strings in multiple places; consider centralizing the set of valid actions (e.g. constant or enum-like structure) to avoid drift and to make configuration validation clearer.

## Individual Comments

### Comment 1
<location> `beetsplug/badfiles.py:230-232` </location>
<code_context>
+
                     ui.print_(error_line)

+            if found_error and error_action != "ask":
+                return self.handle_import_action(error_action, "error")
+            elif found_warning and warning_action != "ask":
+                return self.handle_import_action(warning_action, "warning")
+
</code_context>

<issue_to_address>
**issue (bug_risk):** Mixed warning/error handling can ignore a stricter warning policy when errors are present.

Given this ordering, when both `found_error` and `found_warning` are true and `error_action != "ask"`, the warning policy is effectively ignored. For instance, with `error_action = "continue"` and `warning_action = "abort"`, execution will continue despite the abort-on-warning configuration. If that’s unintended, consider resolving actions by severity (e.g., assigning priorities and choosing the most conservative) or clearly documenting that error handling always takes precedence over warning handling.
</issue_to_address>

### Comment 2
<location> `docs/plugins/badfiles.rst:37-38` </location>
<code_context>
 and errors will be presented when selecting a tagging option.

+`import_action_on_error` and `import_action_on_warning` can be used to take
+automatic action on warning and errors. Both options default to `ask`.
+Valid options for both `import_action_on_(warning|error)` are
+`ask skip abort continue`.
</code_context>

<issue_to_address>
**issue (typo):** Fix grammar in "warning and errors" phrase.

The phrase is grammatically inconsistent; please update it to "automatic action on warnings and errors."

```suggestion
`import_action_on_error` and `import_action_on_warning` can be used to take
automatic action on warnings and errors. Both options default to `ask`.
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[tommyschnabel]

Sorry for the forced updates, everything is sync'ed with upstream/master now

[codecov]

Codecov Report

❌ Patch coverage is 22.22222% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.00%. Comparing base (3d3f5e3) to head (7e23992).

Files with missing lines	Patch %	Lines
beetsplug/badfiles.py	22.22%	16 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6295      +/-   ##
==========================================
- Coverage   72.07%   72.00%   -0.07%     
==========================================
  Files         159      159              
  Lines       20633    20660      +27     
  Branches     3273     3281       +8     
==========================================
+ Hits        14871    14877       +6     
- Misses       5053     5069      +16     
- Partials      709      714       +5     

Files with missing lines	Coverage Δ
beetsplug/tidal/__init__.py	85.32% <ø> (ø)
beetsplug/badfiles.py	27.77% <22.22%> (-1.12%)	⬇️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[tommyschnabel]

@snejus What's the proper way to request a review?

[snejus]

@tommyschnabel review is automatically requested from @beetbox/maintainers when you submit a PR! You want to fix the issues with the CI now, I think.

[tommyschnabel]

This is ready to be reviewed now 😄

[snejus]

Would you mind adding Literal types here for clarity?

[snejus]

You should use confuse library to validate that you're getting confuse.OneOf the four options in each case (lines 212-213). This will remove the need for this logic.

[snejus]

You should define a mapping

action_name_by_action = {"abort": "Aborting", ...}

and then deduplicate ui.print statements:

        ui.print_(
           f"{ui.colorize('text_warning', action_name_by_action[action])}"
            f" due to import_action_on_{failure_type} configuration"
        )
        if action == "abort":
            raise importer.ImportAbortError()
        if action == "skip":
            return importer.Action.SKIP
        return None

[snejus]

This should be undone as it's unrelated.

[snejus]

You should add default configuration inside __init__:

self.config.add(
    {
        "import_action_on_warning": "ask",
        ...,
    }
)

See other plugins for examples.