Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix match option to only consider basename when given a path argument #550

Merged
merged 5 commits into from
Dec 30, 2021

Conversation

oczkoisse
Copy link
Contributor

@oczkoisse oczkoisse commented Aug 16, 2021

Fixes #549.

This uses os.path.basename before trying to match against the pattern set in --match option. This way, even if full paths to files are passed to pydocstyle (by VSCode, for example), only filename part of the path will be matched with the set pattern, thereby eliminating accidental matches with the path. This is also more consistent with how --match is documented previously.

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
    If you've added an error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

src/pydocstyle/config.py Show resolved Hide resolved
src/pydocstyle/config.py Show resolved Hide resolved
docs/release_notes.rst Show resolved Hide resolved
@sambhav
Copy link
Member

sambhav commented Dec 30, 2021

@oczkoisse 👋 Thank you so much for the fix and sorry for the delay in reviewing the PR. After my latest commit e5880d5 (#550) in order to preserve b/w compat for match patterns like (src\/*\.py) i.e patterns that match a full path for eg. - it seems like it breaks the test you added for the negative regex matches.

I am in a difficult conundrum with this since one way breaks b/w compat for nested match paths and the other causes issues with negative match directives.

Re-thinking this PR in the context of #529 - I think it might be better for me to revert my latest commit and merge #529 to have proper support for nested dir paths.

@sambhav sambhav enabled auto-merge (squash) December 30, 2021 21:13
@sambhav sambhav merged commit bd49933 into PyCQA:master Dec 30, 2021
@oczkoisse
Copy link
Contributor Author

Thanks @samj1912 for taking the time to review this PR. I think #529 is a good way to provide an alternative to people who rely on --match to match against nested paths.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--match should consider only basename when given a path argument
2 participants