Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: strings: add new functions to deprecate strings.{Trim/TrimRight/TrimLeft} to avoid mistakes #50399

Open
hitzhangjie opened this issue Dec 31, 2021 · 3 comments

Comments

@hitzhangjie
Copy link
Contributor

hitzhangjie commented Dec 31, 2021

Hi, could we add new methods to solve the problem of cutset type in Strings.{Trim/TrimRight/TrimLeft}.

Apparently, many gohpers misunderstood the this cutset type, and it cauese some bugs. And, there're several proposals for solving the cutset type problem:

I read through this proposals and see the conclusion is no change will be made.

This cutset problem affects many developers:

  • if we have known the problem already, we may not make the same mistake again.
  • but if we doesn't know it in advance, we may likely make the mistake.

In python, str.{strip/rstrip/lstrip}('cutset') method works the same as in go strings.{Trim/TrimRight/TrimLeft}(s, "cutset"). Maybe this signature is ok.

If we:

  • add new methods strings.{TrimCutset\TrimRightCutset\TrimLeftCutset}, and add //Deprecated and go:linkname on strings.{Trim/TrimRight/TrimLeft}, we may make this clearer
  • or add new methods strings.{TrimCutset\TrimRightCutset\TrimLeftCutset}, and declare the cutset as []rune, and use this methods to reimplement strings.{Trim/TrimRight/TrimLeft}
@gopherbot gopherbot added this to the Proposal milestone Dec 31, 2021
@hitzhangjie hitzhangjie changed the title proposal: strings.{Trim/TrimRight/TrimLeft} change cutset type from string []rune proposal: add new methods strings.{Strip/StripRight/StripLeft} to deprecate strings.{Trim/TrimRight/TrimLeft} to define cutset as []rune Dec 31, 2021
@hitzhangjie hitzhangjie changed the title proposal: add new methods strings.{Strip/StripRight/StripLeft} to deprecate strings.{Trim/TrimRight/TrimLeft} to define cutset as []rune proposal: add new methods to deprecate or reimplement strings.{Trim/TrimRight/TrimLeft} to define cutset as []rune Dec 31, 2021
@hitzhangjie hitzhangjie changed the title proposal: add new methods to deprecate or reimplement strings.{Trim/TrimRight/TrimLeft} to define cutset as []rune proposal: add new methods to deprecate or reimplement strings.{Trim/TrimRight/TrimLeft} to avoid mistakes Dec 31, 2021
@martisch
Copy link
Contributor

martisch commented Dec 31, 2021

I dont see how using []rune will avoid the duplicate characters in cutset issues as the user now may as well define duplicate elements in []rune. In addition []rune has the downside of not being a 1 to 1 replacement of the old function signature and has larger storage requirements and is not constant.

I dont think Strip conveys a meaning that it is a cutset more or less then Trim so I dont think just advising to use Strip instead of Trim makes developers more or less likely to mistake the function as not using a cutset.

@hitzhangjie
Copy link
Contributor Author

@martisch I edit the issue again to focus on 1 suggestion. Thanks.

@ianlancetaylor ianlancetaylor changed the title proposal: add new methods to deprecate or reimplement strings.{Trim/TrimRight/TrimLeft} to avoid mistakes proposal: strings: add new functions to deprecate strings.{Trim/TrimRight/TrimLeft} to avoid mistakes Dec 31, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Dec 31, 2021
@ianlancetaylor
Copy link
Contributor

Presumably if we do something in the strings package we would also it in the bytes package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

4 participants