Skip to content

Provide support for ConfigItem to manage a regex pattern of keys #522

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

Conversation

nathanjmcdougall
Copy link
Owner

So far, have Implement regex-based key interface

Copy link

codspeed-hq bot commented Apr 17, 2025

CodSpeed Performance Report

Merging #522 will not alter performance

Comparing 505-generalize-configitem-to-allow-it-to-define-a-family-of-multiple-items (c4c4004) with main (d7851f4)

Summary

✅ 1 untouched benchmarks

Copy link

codecov bot commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 84.93151% with 11 lines in your changes missing coverage. Please review.

Project coverage is 97.27%. Comparing base (d7851f4) to head (c4c4004).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/usethis/_integrations/file/ini/io_.py 83.01% 9 Missing ⚠️
src/usethis/_integrations/file/toml/io_.py 87.50% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (84.93%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
- Coverage   97.51%   97.27%   -0.24%     
==========================================
  Files          83       83              
  Lines        3901     3966      +65     
==========================================
+ Hits         3804     3858      +54     
- Misses         97      108      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nathanjmcdougall nathanjmcdougall marked this pull request as ready for review April 17, 2025 06:25
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces regex support for configuration keys by updating the key interface across multiple file managers and tests. Key changes include:

  • Adding new tests for regex-based key handling in both setup.cfg and INI files.
  • Extending the Key alias and print_keys helper to support compiled regex patterns.
  • Implementing regex validation in the TOML and INI file manager integrations.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/usethis/test_usethis_tool.py Added tests to verify regex-based config insertion and removal in setup.cfg.
tests/usethis/integrations/file/ini/test_ini_io.py Introduced tests for deleting options and sections via regex matching.
src/usethis/_io.py Updated the Key type alias and print_keys to handle regex patterns.
src/usethis/integrations/file/toml/io.py Added regex validation in key handling and error messages for unsupported regex keys.
src/usethis/integrations/file/ini/io.py Enhanced regex support in key and section deletion, with stricter type checks.

elif isinstance(key, re.Pattern):
msg = (
f"Regex-based keys are not supported in TOML files: "
f"{print_keys([*so_far_keys, key])}"
Copy link
Preview

Copilot AI Apr 17, 2025

Choose a reason for hiding this comment

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

Consider clarifying in the error message or documentation that regex-based keys are intentionally not supported in TOML files to avoid potential confusion for users expecting similar regex support as in INI configurations.

Suggested change
f"{print_keys([*so_far_keys, key])}"
f"{print_keys([*so_far_keys, key])}. "
"This is an intentional design decision to ensure compatibility with TOML standards."

Copilot uses AI. Check for mistakes.

Copy link
Owner Author

@nathanjmcdougall nathanjmcdougall Apr 17, 2025

Choose a reason for hiding this comment

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

I've tweaked this a bit, but I agree it's sensible to add a bit more explicit detail.

@nathanjmcdougall nathanjmcdougall merged commit 868463e into main Apr 17, 2025
16 of 17 checks passed
@nathanjmcdougall nathanjmcdougall deleted the 505-generalize-configitem-to-allow-it-to-define-a-family-of-multiple-items branch April 17, 2025 06:42
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.

Generalize ConfigItem to allow it to define a family of multiple items
1 participant