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

Use match? when regexp match data is unused #7263

Merged

Conversation

segiddins
Copy link
Member

Improved performance / reduced allocations

What was the end-user or developer problem that led to this PR?

What is your fix for the problem, implemented in this PR?

Make sure the following tasks are checked

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Surprised Performance/RegexpMatch does not catch this!

@segiddins segiddins force-pushed the segiddins/use-match-when-regexp-match-data-is-unused branch from 76d404b to 09d259e Compare December 13, 2023 16:58
@martinemde
Copy link
Member

One of the methods ... moved, I think? Want to correct that here or just resolve the conflict?

@deivid-rodriguez
Copy link
Member

Yeah, the method was deleted at #7258, we'd want to apply the same patch, but here:

##
# Corrects +path+ (usually returned by `URI.parse().path` on Windows), that
# comes with a leading slash.
def self.correct_for_windows_path(path)
if path[0].chr == "/" && path[1].chr =~ /[a-z]/i && path[2].chr == ":"
path[1..-1]
else
path
end
.

Improved performance / reduced allocations
@deivid-rodriguez deivid-rodriguez force-pushed the segiddins/use-match-when-regexp-match-data-is-unused branch from 09d259e to b04726c Compare December 13, 2023 20:55
@deivid-rodriguez
Copy link
Member

I resolved the conflict 👍.

@martinemde martinemde merged commit 3285550 into master Dec 13, 2023
80 checks passed
@martinemde martinemde deleted the segiddins/use-match-when-regexp-match-data-is-unused branch December 13, 2023 22:00
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.

None yet

3 participants