Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add parameterless overloads of TrimStart/TrimEnd#8834

Merged
joperezr merged 3 commits intodotnet:masterfrom
thiagocamargos:issue6485
Jan 11, 2017
Merged

Add parameterless overloads of TrimStart/TrimEnd#8834
joperezr merged 3 commits intodotnet:masterfrom
thiagocamargos:issue6485

Conversation

@thiagocamargos
Copy link
Copy Markdown

@thiagocamargos thiagocamargos commented Jan 6, 2017

Add parameterless overloads of TrimStart/TrimEnd on String.Manipulation.cs, model.xml and ref/mscorlib.cs

Fix https://github.com/dotnet/corefx/issues/6485

@dnfclas
Copy link
Copy Markdown

dnfclas commented Jan 6, 2017

Hi @thiagocamargos, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

@dnfclas
Copy link
Copy Markdown

dnfclas commented Jan 6, 2017

@thiagocamargos, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

Copy link
Copy Markdown
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @thiagocamargos thanks for your contribution!

LGTM apart from that small comment I gave. We will need to add the appropiate tests into corefx once we expose this.


// Removes a set of characters from the beginning of this string.
public String TrimStart() {
return TrimHelper(TrimHead);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: You could simplify this by simply doing:

public string TrimStart() => TrimHelper(TrimHead);
public string TrimEnd() => TrimHelper(TrimTail);

@thiagocamargos
Copy link
Copy Markdown
Author

Hi @joperezr
I see there are already some tests on https://github.com/dotnet/corefx/blob/master/src/System.Runtime/tests/System/StringTests.cs#L2346 and https://github.com/dotnet/corefx/blob/master/src/System.Runtime/tests/System/StringTests.cs#L2328
that used the TrimStart/End(params char []) overload and will use now the parameterless overload.

Should I adapt the test to ensure it will also keep calling the params char [] overload when trimChars is null?

@joperezr
Copy link
Copy Markdown
Member

Should I adapt the test to ensure it will also keep calling the params char [] overload when trimChars is null

Yes, I believe you should ensure it keeps calling the other params one so that we don't loose coverage there and perhaps add some basic ones for this two methods as well.

@joperezr joperezr merged commit f00766b into dotnet:master Jan 11, 2017
@joperezr
Copy link
Copy Markdown
Member

@thiagocamargos thanks for your contribution!

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Jan 12, 2017

@thiagocamargos Would you mind porting this change to CoreRT (https://github.com/dotnet/corert/tree/master/src/System.Private.CoreLib/src) as well?

@thiagocamargos
Copy link
Copy Markdown
Author

Hi @jkotas
Sure, I'll do that.

manofstick pushed a commit to manofstick/coreclr that referenced this pull request Jan 16, 2017
Add parameterless overloads of TrimStart/TrimEnd
@justinvp
Copy link
Copy Markdown

justinvp commented Jan 25, 2017

@thiagocamargos, I included these in dotnet/corert#2587 while I was porting my changes from #9009. Apologies if you were already working on it.

@thiagocamargos
Copy link
Copy Markdown
Author

@justinvp no problem, I could not work on last couple weeks

@thiagocamargos thiagocamargos deleted the issue6485 branch March 14, 2017 23:07
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Add parameterless overloads of TrimStart/TrimEnd

Commit migrated from dotnet/coreclr@f00766b
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants