Project

General

Profile

Actions

Feature #21346

open

Introduce `String#ensure_suffix`

Added by matheusrich (Matheus Richard) 21 days ago. Updated about 9 hours ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:122154]

Description

Problem

Ensuring a string has a specific suffix or prefix is a common operation in many applications.
Bundler itself uses it:

https://github.com/rubygems/rubygems/blob/d409ec8b5fc647fabe30e37e17cd1ea857634f6b/bundler/lib/bundler/uri_normalizer.rb#L17

Here are GitHub search queries that might find this pattern in other places:

  1. for Ruby: /end(?:s)?_with\?\(['"].*['"]\) \?/ lang:ruby -is:fork
  2. 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 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 with end_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.
Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0Like1Like0Like0Like0Like0Like0Like0Like1Like0Like0Like0Like0