Proposal: Add support for list of strings in String.{trim, trim_leading, trim_trailing}

[eksperimental]

Hello everyone,

Currently the second argument of funtions String.{trim, trim_leading,
trim_trailing}/2 can only be a string.
I would like Elixir to support a list of strings to be trimmed, such as:

String.trim_leading(tag_name, ["OTP_", "OTP-"])

There is a precedent of other functions taking list of strings as in
the case of `String.starts_with?/2`, `String.end_with?/2` and
`String.contains?/2`.

Currently it can be achieved with `String.replace_leading/3` and
`String.replace_trailing/3` but:
1. by using "" as the replacement it is not as clear as what trim
conveys.
2. There is no direct replacement for `String.trim` and two
function calls are needed:
string |> String.replace_leading([" ", "-", "_"], "") |>
String.replace_trailing([" ", "-", "_"], "")

where with this would suffice.
String.trim(string, [" ", "-", "_"])

Please let me know what you think about it.

[José Valim]

Yes, I think those would be consistent additions!

[eksperimental]

On Tue, 25 Jan 2022 18:33:08 +0100
José Valim <jose....@dashbit.co> wrote:

> Yes, I think those would be consistent additions!
>

I will try to submit a PR this week then.

[eksperimental]

When starting to work in this implementation was wondering what should
be the return value of

String.trim_leading("--__--abc", [" ", "-", "_"])


and I realized that it should mimic String.replace_leading/3
Therefore it should returning "abc"

Which leads me to think we should introduce String.trim_prefix/2, in
order to be consistent with String.replace_prefix/3
where the same input as above should behave like this:

iex> String.trim_prefix("--__--abc", [" ", "-", "_"])
"__--abc"

Which also makes me think that String.replace_prefix should also
behave like String.replace/3 where the second argument (match) is a
pattern, not a string (ie. a string, list of strings, or a compiled
search pattern). By introducing this last consistency change, the
implementation of the consistency changes for the other functions
mentioned above is straight-forward.

So in short this consistency proposals suggests some functions to
support t:String.pattern/0 which could be a string, a list of strings,
or a compiled search pattern.

1. `String.replace_{leading, prefix, suffix, trailing}/3` to support in
its second argument (match) t:String.pattern/0. Current
String.replace/4 supports t:String.pattern/0 | t:Regex.t/0, but for
the sake of simplifying this proposal I would leave out
Regexes out of the equation for now.

2. Introduce String.trim_{prefix, suffix}/2 which will support
t:String.pattern/0 as a its second arguments. These functions should
mimic String.replace_{prefix, suffix}/2

3. Add support for t:String.pattern/0 for String.{trim, trim_leading,
trim_trailing}/2 to support t:String.pattern/0 as its second argument.
These functions should mimic String.{replace, replace_leading,
replace_trailing}/3

Please let me know what you think.

[José Valim]

Thanks for the proposal. I believe we don't support all patterns because it will be inefficient, especially the suffix/trailing functions. It is not necessarily trivial to implement it for regexes either (and regexes have their own mechanism to represent prefixes/suffixes).

The reason we don't have "trim_prefix" and "trim_suffix" is because there is no reasonable implementation of trim_prefix/1 and trim_suffix/1 - where no pattern to trim on is given (for example, it it to the behaviour of trim_leading/1 and trim_trailing/1). So we could add trim_prefix/2 and trim_suffix/2, but then it is inconsistent with other trim* functions. So unless there is a strong reason for adding this, rather than consistency, I would not go this route.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/621f703a.1c69fb81.2b06.fa97SMTPIN_ADDED_MISSING%40gmr-mx.google.com.

[eksperimental]

> So unless there is a strong
> reason for adding this, rather than consistency, I would not go this
> route.

Personally I don't think it is an inconsistency because there is no
equivalent of String.trim_leading/1 (or the equivalent makes no sense,
or hardly has any use).

The need for String.trim_prefix/2 is because what actually solves the
problem (my use case) that I initially shows.

String.trim_leading could be considered an addition for consistency,
but I guess it could still be useful in some cases but its
implementation would not be as optimal as Sting.trim_prefix/2' s.

[José Valim]

Sorry, I got so wrapped up on the proposal that I forgot about your example. :D

We can make replace_leading/trailing/prefix/suffix accept t | [t] and that would solve your use case. We already accept only this subset on starts_with? / ends_with?, so I think it is a natural extension here.

Then it will automatically work on trim_leading/trim_trailing, since those just use the replace equivalents. Shall we go this route for now? Please open up an issue and feel free to submit a PR. :)

I am not convinced on trim_prefix/trim_suffix yet though, but you can use the replace functions to achieve the same.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/621fb346.1c69fb81.37fc8.dec4SMTPIN_ADDED_MISSING%40gmr-mx.google.com.

[eksperimental]

Well, all these questions came up when I started implementing a
solution, and the further I looked the higher I climbed up the branches.

I will limit the scope to string and list of strings as you suggest,
later we can extend it if necessary.

If i see the need to implement a trim_{prefix, suffix}/2 I will make it
private then.

I will create a an issue and submit a PR when I have a WIP version of a
it.

Cheers

On Wed, 2 Mar 2022 19:56:42 +0100