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

Change Regex::MatchData#to_s to return matched substring #14115

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

Vendicated
Copy link
Contributor

@Vendicated Vendicated commented Dec 19, 2023

Related issue: #13118

Currently, MatchData#to_s is the same as MatchData#inspect, which means that it returns an object representation of the MatchData: Regex::MatchData("ys" 1:"y" 2:"s")

This is not only inconsistent with Ruby, which returns the matched substring, and most other languages, like rust's regexp crate, but also not really useful for anything besides debugging (which is exactly where you should use inspect over to_s).

Much more useful would be for it to return the matched substring. Currently, the only way to do so is via match[0], but that's unintuitive and confusing

Thus, this PR changes #to_s to do just that. This is a breaking change, but I doubt many codebases actually use or rely on the old to_s. #inspect still has the old behaviour.

Resolves part of #13118

@Vendicated
Copy link
Contributor Author

CI fail seems to be unrelated

     1) UDPSocket using IPv4 joins and transmits to multicast groups
  
         Could not bind to '0.0.0.0:37747': Address already in use (Socket::BindError)

@straight-shoota straight-shoota changed the title RegEx::MatchData#to_s: Return matched substring instead of #inspect Change Regex::MatchData#to_s to return matched substring Dec 19, 2023
@straight-shoota straight-shoota added this to the 1.11.0 milestone Dec 20, 2023
@straight-shoota straight-shoota merged commit 691677f into crystal-lang:master Dec 21, 2023
55 checks passed
@Vendicated Vendicated deleted the matchData#to_s branch December 21, 2023 23:18
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.

6 participants