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

Preserves interpreting multi-line strings as a single value for exclude in TOML files #11828

Merged

Conversation

posita
Copy link
Contributor

@posita posita commented Dec 22, 2021

Introduces an alternative where multiple regexes can be expressed as an array. Includes additional unit tests.

This preserves backward compatibility by allowing multi-line values in TOML to be treated as a single regex for those who have come to rely on that while exposing an array-based expression as a more ergonomic (and TOML-like) alternative.

Fixes #11825. Obsoletes (or is obsoleted by) #11746.

@posita
Copy link
Contributor Author

posita commented Dec 22, 2021

Doc screenshots:

Screen Shot 2021-12-22 at 17 44 29

… links to the section that contains …

Screen Shot 2021-12-22 at 17 46 29

Created with: make -C docs clean html. I have verified that make -C docs clean linkcheck does not surface any errors related to this change. (Although there are unrelated errors.)

@posita posita force-pushed the posita/0/multiple-excludes-in-toml-fix-11825 branch 2 times, most recently from 0fd2c8e to f483ee1 Compare December 22, 2021 23:45
@posita posita changed the title Allows for exclude as a multiline basic string in TOML files Allows for exclude as a multi-line basic string in TOML files Dec 22, 2021
@@ -143,6 +149,7 @@ def check_follow_imports(choice: str) -> str:
'disable_error_code': try_split,
'enable_error_code': try_split,
'package_root': try_split,
'exclude': str_or_seq_of_strs_as_list_of_strs,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function name is a bit verbose. I'm open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed and don't have a supremely better idea. Maybe into_list - implying that list is the result, but not implying that splitting is happening.

@posita posita changed the title Allows for exclude as a multi-line basic string in TOML files Preserves interpreting multi-line strings as a single value for exclude in TOML files Dec 22, 2021
@nipunn1313
Copy link
Contributor

This looks great!

@posita posita force-pushed the posita/0/multiple-excludes-in-toml-fix-11825 branch from f483ee1 to 66c0b7d Compare December 23, 2021 00:29
Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Are these comments cleared or not?

docs/source/config_file.rst Outdated Show resolved Hide resolved
[tool.mypy]
exclude = [
'^file1\.py$', # literal (no escaping)
"^file2\\.py$", # basic (backslash needs escaping)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"^file2\\.py$", # basic (backslash needs escaping)
"^windows_dir\\file2\.py$", # a literal backslash should be escaped with a backslash

Copy link
Contributor Author

@posita posita Dec 23, 2021

Choose a reason for hiding this comment

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

See my comment above.

Multiple regexes are expressed as a sequence.

Fixes python#11825.
@posita posita force-pushed the posita/0/multiple-excludes-in-toml-fix-11825 branch from 66c0b7d to bbd5431 Compare December 23, 2021 05:58
docs/source/config_file.rst Show resolved Hide resolved
docs/source/config_file.rst Show resolved Hide resolved
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Thank you @posita and @mawillcockson for fixing this!

I brought this up in #11700, so hopefully we get a 0.931 that adds this (and whatever we decide for #11830)

@posita
Copy link
Contributor Author

posita commented Jan 1, 2022

This looks good to me.

Thank you @posita and @mawillcockson for fixing this!

I brought this up in #11700, so hopefully we get a 0.931 that adds this (and whatever we decide for #11830)

@hauntsaninja, see #11881 and this comment (in case you wanted to update this).

@JukkaL JukkaL merged commit 246c9ee into python:master Jan 4, 2022
@JukkaL
Copy link
Collaborator

JukkaL commented Jan 4, 2022

Thank you for fixing this!

JukkaL pushed a commit that referenced this pull request Jan 5, 2022
…ude` in TOML files (#11828)

Multiple regexes are expressed as a sequence.

Fixes #11825.

Co-authored-by: Matthew W <[email protected]>
Co-authored-by: Shantanu <[email protected]>
@posita posita deleted the posita/0/multiple-excludes-in-toml-fix-11825 branch January 6, 2022 14:35
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
…ude` in TOML files (python#11828)

Multiple regexes are expressed as a sequence.

Fixes python#11825.

Co-authored-by: Matthew W <[email protected]>
Co-authored-by: Shantanu <[email protected]>
@ghost ghost mentioned this pull request Jan 26, 2022
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.

Crash/Regression - 0.930 no longer accepts multiline basic strings for exclude patterns in pyproject.toml
6 participants