Feature #21346
openIntroduce `String#ensure_suffix`
Description
Problem¶
Ensuring a string has a specific suffix or prefix is a common operation in many applications.
Bundler itself uses it:
Here are GitHub search queries that might find this pattern in other places:
- for Ruby:
/end(?:s)?_with\?\(['"].*['"]\) \?/ lang:ruby -is:fork
- for Crystal (a language very similar to Ruby):
/ends_with\?\(['"].*['"]\) \?/ lang:crystal -is:fork
Suggested solution¶
I believe Ruby would benefit from having a first-class method for this purpose.
I suggest the String#ensure_suffix
and String#ensure_prefix
methods.
I think these names are intuitive enough (here are 2 examples of people using ensure
for this purpose (1, 2)).
I've gone ahead and implemented String#ensure_suffix
in a pull request but the suggested behavior is this:
"Hell".ensure_suffix("o!") # => "Hello!"
"Hello!".ensure_suffix("o!") # => "Hello!"
s = "Hello!"
s.ensure_suffix("!").equal?(s) # => true # returns same object if already suffixed
Updated by matheusrich (Matheus Richard) 21 days ago
If approved, I can add String#ensure_prefix
and the bang versions of those methods.
Updated by duerst (Martin Dürst) 21 days ago
You say "queries that might find this pattern". That seems to say that you haven't found it yet.
What's the result for
"Hello".ensure_suffix("o!")
Is it "Helloo!", or is it "Hello!"?
Updated by matheusrich (Matheus Richard) 20 days ago
· Edited
You say "queries that might find this pattern". That seems to say that you haven't found it yet.
I'm not sure what you mean. I just meant that the regex can find this pattern, but I can't guarantee it will only find that.
What's the result for
"Hello".ensure_suffix("o!")
Is it "Helloo!", or is it "Hello!"?
As currently implemented, it returns "Helloo!"
Updated by nobu (Nobuyoshi Nakada) 20 days ago
matheusrich (Matheus Richard) wrote in #note-3:
You say "queries that might find this pattern". That seems to say that you haven't found it yet.
I'm not sure what you mean. I just meant that the regex can find this pattern, but I can't guarantee it will only find that.
Maybe like this?
"Hell".sub(/(?<!o!)\z/, "o!") #=> "Hello!"
"Hello!".sub(/(?<!o!)\z/, "o!") #=> "Hello!"
"o!Hell".sub(/(?<!o!)\z/, "o!") #=> "o!Hello!"
What's the result for
"Hello".ensure_suffix("o!")
Is it "Helloo!", or is it "Hello!"?
As currently implemented, it returns
"Helloo!"
You should describe the corner case in the doc, and the test preferably.
Updated by matheusrich (Matheus Richard) 19 days ago
@nobu (Nobuyoshi Nakada) added! Thanks for the review.
Updated by Eregon (Benoit Daloze) 5 days ago
I think this would be good to add (both methods).
Updated by matheusrich (Matheus Richard) 4 days ago
Related: I've proposed both methods to Crystal, and the recently were approved and merged.
Updated by maxfelsher (Max Felsher) 3 days ago
I like this idea, but I'm concerned about the same method returning self
in one case and a new string in another. It seems like it'd be easier to reason about if it either always returned a new string or always affected the original object. Or if there's a bang version, it would be consistent with chomp
, sub
, etc., for the non-bang version to always return a new string and the bang version to always affect the original object.
Apologies if I'm missing something!
Updated by matz (Yukihiro Matsumoto) 1 day ago
ensure_suffix
accepted. IMO, it should always copy the original string, even when it ends with the suffix. I don't see any reason to add bang-version, right now. Keep it on-demand (because we are lazy).
Matz.
Updated by Eregon (Benoit Daloze) 1 day ago
What about ensure_prefix
?
I think it's good to have parity here, I would be surprised if that doesn't exist but ensure_suffix
does.
Also we have start_with?
/end_with?
, etc.
Updated by matheusrich (Matheus Richard) 1 day ago
I've updated the PR as per Matz's comment.
Updated by ufuk (Ufuk Kayserilioglu) 1 day ago
I personally find the "Hello".ensure_suffix("o!")
case returning "Helloo!"
very unexpected. I would have expected the change in the string to be the minimal operation needed to ensure that the string ends with the given suffix (i.e. for the result of the example to be "Hello!"
), but this isn't what's happening.
@matz (Yukihiro Matsumoto) Is this the behaviour that you want?
Updated by matheusrich (Matheus Richard) 1 day ago
@ufuk (Ufuk Kayserilioglu) see dev meeting discussion
matz: "Hello".ensure_suffix("o!") should return "Helloo!"
Updated by ufuk (Ufuk Kayserilioglu) 1 day ago
matheusrich (Matheus Richard) wrote in #note-13:
@ufuk (Ufuk Kayserilioglu) see dev meeting discussion
matz: "Hello".ensure_suffix("o!") should return "Helloo!"
Ah, ok, I hadn't read the dev meeting discussion. Thanks for the pointer.
Updated by mame (Yusuke Endoh) 1 day ago
Let me record the background of the discussion. During the dev meeting, we considered the following use case:
path.ends_with?(".rb") ? path : "#{path}.rb"
When rewriting this as path.ensure_suffix(".rb")
, the behavior of appending just "b"
when path is "foo.r"
to make it "foo.rb"
is somewhat unexpected.
This is the rationale behind the idea that "Hello".ensure_suffix("o!")
should return "Helloo!"
.
That said, on a personal note, I think the method name ensure_suffix
is indeed confusing. As @ufuk (Ufuk Kayserilioglu) pointed out, and as @akr (Akira Tanaka) also mentioned in the meeting, it sounds like a method that makes the minimal necessary addition. I agree with you.
The code snippet path.ends_with?(".rb") ? path : "#{path}.rb"
isn't particularly long, and its meaning is quite clear, so maybe we don't need this method at all.
Updated by matheusrich (Matheus Richard) about 9 hours ago
I think path.ends_with?(".rb") ? path : "#{path}.rb"
is much more common than the version that just appends b
to "foo.r"
. IMO we should optimize for the common operation, which is more predictable and simple as well.
Updated by matheusrich (Matheus Richard) about 9 hours ago
Some alternative names:
with_suffix
-
end_with
: pairs well withend_with?
, but might be too similar and error prone? -
add_suffix
: I think it implies that the suffix is always added, which is not true.