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

GH-77609: Support following symlinks in pathlib.Path.glob() #104176

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented May 4, 2023

Add support for *** wildcard in pathlib.Path.glob() and rglob(). This wildcard works like **, except that it also recurses into symlinks.

See Recursive Globbing in the zshexpn manpage for prior art.

Alternative to #102616

@barneygale
Copy link
Contributor Author

Marking as a draft because it would be good to land #104292 and #104141 first.

@barneygale barneygale marked this pull request as draft May 8, 2023 14:47
@zooba
Copy link
Member

zooba commented May 8, 2023

How does this help us fix the mixed up behaviour of the current function?

@barneygale
Copy link
Contributor Author

I don't think the current behaviour is wrong, just difficult to optimize. With this PR, users would be able to recursively walk symlinks, and we'd be able to optimize ***/*.py later on (but not **/*.py). Still, I'm not convinced this is the right way forward.

@zooba
Copy link
Member

zooba commented May 8, 2023

Still, I'm not convinced this is the right way forward.

I'm surprised, because you closed the other PR saying you were convinced this was the way forward.

Personally, I preferred the other way. It's a bit more work, but in ~5 years time we'll have forgotten about it and will just have sensible defaults. Adding this way and leaving the other one leaves us with two warts (okay, maybe 1.5, if people like this, but I feel like special patterns specifically for symlinks is too much).

@barneygale
Copy link
Contributor Author

Still, I'm not convinced this is the right way forward.

I'm surprised, because you closed the other PR saying you were convinced this was the way forward.

I know. I literally changed my mind again last night while playing around with the glob() implementation! :P

Personally, I preferred the other way. It's a bit more work, but in ~5 years time we'll have forgotten about it and will just have sensible defaults. Adding this way and leaving the other one leaves us with two warts (okay, maybe 1.5, if people like this, but I feel like special patterns specifically for symlinks is too much).

👍 fair point, thanks for the advice.

@barneygale
Copy link
Contributor Author

Closing in favour of #102616

@barneygale barneygale closed this May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants