Runtime: Add string.RemovePrefix/RemoveSuffix (dupe)

[grant-d]

[Picking up an item from up-for-grabs]

It is useful to have methods that trim a specified prefix or suffix from a string. This is somewhat simple for a developer to write themselves, but it seems to be a common enough request that it would be useful to support directly, especially in regards to robustness and performance.

Here are some references gleaned from the original issue.
http://stackoverflow.com/questions/7170909/trim-string-from-end-of-string-in-net-why-is-this-missing
http://stackoverflow.com/questions/4101539/c-sharp-removing-strings-from-end-of-string
http://stackoverflow.com/questions/5284591/how-to-remove-a-suffix-from-end-of-string
http://stackoverflow.com/questions/4335878/c-sharp-trimstart-with-string-parameter

Usage

There are only 2 overloads for each method, as shown in the following examples:

// Default overload
"http://foo.com".TrimPrefix("http://").TrimSuffix(".com") == "foo"

// StringComparison
"http://foo.com".TrimPrefix("HTTP://", StringComparison.OrdinalIgnoreCase) == "foo.com"

Background

• Original issue: https://github.com/dotnet/corefx/issues/1244
• Api review

Proposed API

/// <summary>
/// Removes the specified prefix if it is found at the start of the string.
/// String comparison uses <see cref="StringComparison.Ordinal"/>.
/// </summary>
/// <param name="prefix">The prefix to remove.</param>
public string RemovePrefix(string value);

/// <summary>
/// Removes the specified prefix if it is found at the start of the string.
/// String comparison uses the specified <see cref="StringComparison"/>.
/// </summary>
/// <param name="suffix">The prefix to remove.</param>
/// <param name="comparisonType">The string comparison method to use.</param>
public string RemovePrefix(string value, StringComparison comparisonType);

/// <summary>
/// Removes the specified suffix if it is found at the end of the string.
/// String comparison uses <see cref="StringComparison.Ordinal"/>.
/// </summary>
/// <param name="suffix">The suffix to remove.</param>
public string RemoveSuffix(string value);

/// <summary>
/// Removes the specified suffix if it is found at the end of the string.
/// String comparison uses the specified <see cref="StringComparison"/>.
/// </summary>
/// <param name="suffix">The suffix to remove.</param>
/// <param name="comparisonType">The string comparison method to use.</param>
public string RemoveSuffix(string value, StringComparison comparisonType);

Details, Decisions, Questions

Some of these items from @tarekgh's feedback: https://github.com/dotnet/corefx/issues/1244#issuecomment-261606399

• Decision: namespace System
  Decision: No bool repeat overloads. The callsite can call this recursively (at the risk of more allocations).
  
  
  
  • Looking that linked StackOverflow questions it seems folks want the non-repeating behavior, i.e. "SIdId".RemoveSuffix("Id") should return "SId", not "S".
  
  
  • We don't want to introduce overloads for TrimEnd and TrimStart that have a non-repeating behavior because it's inconsistent with the existing ones.
  
  
  • At the same time, we feel the bool option is overkill and not very readable from the call site.
  
  
• Should be instance methods on String (as opposed to extension methods)
• Should we add corresponding methods to TextInfo (or whatever they care called in globalization)?

POC

This is a relatively simple addition, POC code below:

/// <summary>
/// Removes the specied prefix if it is found at the start of the string.
/// String comparison uses <see cref="StringComparison.Ordinal"/>.
/// </summary>
/// <param name="suffix">The prefix to remove.</param>
public string RemovePrefix(string prefix, StringComparison comparisonType)
{
    if (prefix == null)
        throw new ArgumentNullException(nameof(prefix));

    if (prefix.Length == 0)
        return this;

    int len = this.Length - prefix.Length;
    if (len < 0)
        return this;

    var found = StartsWith(prefix, comparisonType);
    if (!found)
        return this;

    if (len == 0)
        return string.Empty;

    var val = InternalSubString(prefix.Length, len);
    return val;
}

/// <summary>
/// Removes the specied suffix if it is found at the end of the string.
/// String comparison uses <see cref="StringComparison.Ordinal"/>.
/// </summary>
/// <param name="suffix">The suffix to remove.</param>
public string RemoveSuffix(string suffix)
    => RemoveSuffix(suffix, StringComparison.Ordinal);

/// <summary>
/// Removes the specied suffix if it is found at the end of the string.
/// String comparison uses the specified <see cref="StringComparison"/>.
/// </summary>
/// <param name="suffix">The suffix to remove.</param>
/// <param name="comparisonType">The string comparison method to use.</param>
public string RemoveSuffix(string suffix, StringComparison comparisonType)
{
    if (suffix == null)
        throw new ArgumentNullException(nameof(suffix));

    if (suffix.Length == 0)
        return this;

    int len = this.Length - suffix.Length;
    if (len < 0)
        return this;

    var found = EndsWith(suffix, comparisonType);
    if (!found)
        return this;

    if (len == 0)
        return string.Empty;

    var val = InternalSubString(0, len);
    return val;
}

[karelz]

Why did we split it off the original issue dotnet/runtime#14386?

[grant-d]

[Edited] Because of the ask for a formal proposal. My understanding of the "example of a good proposal" was that the formal spec should be in the preamble, and not require scrolling through discussion.

I can inline it here if that's preferable for easier tracking?

[danmosemsft]

It's unfortunate this allocates a temporary string: "http://foo.com".TrimPrefix("http://").TrimSuffix(".com") but perhaps it's not common to do both.

Since this original issue was discussed, Span<char> has become available. Should these be added to the extension methods:
https://github.com/dotnet/corefx/blob/master/src/System.Memory/ref/System.Memory.cs#L85

[tarekgh]

Because of the ask for a formal proposal. I can inline it here if that's preferable?

Can't we just update the original proposal? now we have 2 issues tracking the same thing.

[grant-d]

I just added Span.Contains in a separate PR, so happy to follow suit with these methods.

[grant-d]

OK, updated original issue: https://github.com/dotnet/corefx/issues/1244