gleam-lang / stdlib

🎁 Gleam's standard library
https://hexdocs.pm/gleam_stdlib/
Apache License 2.0
492 stars 175 forks source link

Implement regex.replace #638

Closed rado-h closed 4 months ago

rado-h commented 5 months ago

Add a replace function to the regex package. If accepted, this should resolve issue #621

rado-h commented 5 months ago

I agree, the regex as a first argument makes more sense in the context of the module. My initial reasoning was that the function would probably be used more often in a pipeline on a string. However scan and split would also have similar patterns of usage and they already take regex as a first argument, so introducing an inconsistency in replace would only make things confusing.

inoas commented 5 months ago

Lovely, thank you!

I was wondering if the regex should go first, as normally the data structure that has the same name as the module goes first. What do you think?

I think we should not repeat the mistake that elixir did, though here it is less of a problem due to the capture symbol and labels, and we should let the data that is being worked on (the string) be in the first spot.

either these operations are called once, or they are probably called more likely on multiple strings with the same regex and more likely that the same string on multiple regex and even then the return value is a string not a regex

rado-h commented 5 months ago

I wonder if regex.replace and regex.split (and probably regex.scan) would be more suitable for the string module as they are actually string operations that happen to use a regular expression as an argument.

rado-h commented 4 months ago

Thank you! Could you update the changelog too please

Done