Skip to content

Conversation

@scuml
Copy link
Contributor

@scuml scuml commented Jan 29, 2025

Improves the matching mechanism for globs addressed in PR #92

This problem is solved with the new full_match method available in python 3.13. https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.full_match

To make the problem less severe in previous python versions, I made an improved version of fnmatch() that handles the most common */** pattern. (Using onlyfnmatch() does not select files on the top level making things feel very broken.) This way we can get the benefit of improved matching in older versions, without having to rely on an additional globber dependency.

Fixes:
#69
#91

@scuml

This comment was marked as spam.

adamchainz added a commit that referenced this pull request Sep 9, 2025
Fixes #91. Supercedes #92 and #134.

---------

Co-authored-by: Stephen Mitchell <scum@mac.com>
@adamchainz
Copy link
Owner

Thank you for starting this one @scuml . I have just merged a version of it in #166, with you as co-author.

I opted to avoid the fnmatch wrapper, since I wasn't confident it would work in all situations, and it would give a third behaviour that could be hard to debug when upgrading django-watchfiles. Therefore the fix is restricted to Python 3.13+.

I did try to backport everything necessary to make Path.full_match() work, but after copying the extensive functional changes in Python 3.13 for fnmatch, glob, and pathlib, it produced an incorrect result, probably due to uncopied changes to other functions.

@scuml
Copy link
Contributor Author

scuml commented Sep 9, 2025

That's the right call. Behavior too unpredictable otherwise.
You and your incredible work came up in a group conversation at DjangoCon Chicago yesterday. Your contributions are appreciated by so many here.

@adamchainz
Copy link
Owner

Oh, thank you!

image

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.

2 participants