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

Feature Request: limit path->lines result to those matching pattern #285

Open
lbe opened this issue Jul 22, 2023 · 2 comments
Open

Feature Request: limit path->lines result to those matching pattern #285

lbe opened this issue Jul 22, 2023 · 2 comments

Comments

@lbe
Copy link

lbe commented Jul 22, 2023

Limit those lines returned from path->lines to only those lines matching a regexp - path->lines(pattern => qr/Line/).

I have submitted a pull request (lbe/Path-Tiny lines_pattern) for the same.

@karenetheridge
Copy link
Contributor

What is the advantage of doing this over grep qr/$pattern/, $path->lines ? Is there a performance gain? It seems about equivalent in readability.

@lbe
Copy link
Author

lbe commented Jul 22, 2023

Thank you for your comment/question.

Performance gain, if any, would be minimal as it is implemented with grep.

I did evaluate from a performance standpoint. The only option that I saw from that standpoint would be to use a memory map on the read. Incorporating an additional dependency would be counter to the Tiny intent.

I agree that readability it similar. My thought is that adding the pattern argument makes the code more concise and keeps less experience developers away from challenges with grep that people seem to experience. While I'm an old hand at grep, I did trip up a few times in writing the voluminous but simple test cases.

One additional point that I mentioned in the pull request but not the issue is that Path::Iterator::Rule does implement pattern for restricting file names. As such, using a similar argument is consistent with its admittedly more complex cousin.

I debated about creating the pull request to begin with since it is not a "major" enhancement - meaning one that extends a limitation or adds a whole new capability. What pushed me over the edge to write and submit it is that Path::Tiny cannot be sub-classed to add pattern.

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

No branches or pull requests

2 participants