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

Add removeprefix/removesuffix to string #185

Closed
ndmitchell opened this issue Mar 16, 2021 · 4 comments
Closed

Add removeprefix/removesuffix to string #185

ndmitchell opened this issue Mar 16, 2021 · 4 comments

Comments

@ndmitchell
Copy link
Contributor

ndmitchell commented Mar 16, 2021

Someone wanted to change libFoos.so to Foos so they did the (not unreasonable) x.lstrip("lib").rstrip(".so"), which looks like it does that, but completely doesn't (it returns Foo). I then change that to index slicing, in particular [:-len(extension)], but that fails in the corner case of an empty extension. These aren't unreasonable things to do, but the default Starlark operators are full of pitfalls.

Python 3.9 added string.removeprefixand string.removesuffix. I've added them to the Rust Starlark, since they are super helpful and avoid bugs. It would be great to add them to standard Starlark too.

@ndmitchell ndmitchell changed the title strip/lstrip/rstrip are dangerous Add removeprefix/removesuffix to string Mar 16, 2021
facebook-github-bot pushed a commit to facebook/starlark-rust that referenced this issue Mar 16, 2021
Summary: These methods got added in Python 3.9. I've seen two engineers get this wrong without these utilities, so add them. I've also proposed their addition to Starlark in bazelbuild/starlark#185.

Reviewed By: milend

Differential Revision: D27077457

fbshipit-source-id: 6d1eb4a6b1c775fdfa1bcbf62f71809e1dcb44c1
@adonovan
Copy link
Contributor

adonovan commented May 6, 2021

Seems reasonable. I'm amazed they took until 3.9 to appear.

ndmitchell added a commit to ndmitchell/starlark that referenced this issue May 28, 2021
@fmeum
Copy link
Contributor

fmeum commented Feb 13, 2022

@adonovan Given these are already in Rust Starlark, what would I need to do to add them to the spec + Bazel? Happy to work on PRs as I would find these functions very useful.

@adonovan
Copy link
Contributor

Thanks for volunteering. Could you send a PR to change to the spec adjacent to https://github.com/bazelbuild/starlark/blob/master/spec.md#string%C2%B7endswith, which we can review? Once that's approved the maintainers of the various implementations can add the function, which should be very straightforward. I can do the Go one and @brandjon can do Java.

@fmeum
Copy link
Contributor

fmeum commented Feb 15, 2022

I submitted #212 as an update to the spec and have prepared bazelbuild/bazel#14824 to implement it in Java Starlark.

laurentlb pushed a commit to ndmitchell/starlark that referenced this issue Feb 15, 2022
laurentlb pushed a commit that referenced this issue Feb 15, 2022
* #185, add removeprefix/removesuffix to the spec

* Make it clear the prefix/suffix is removed at most once
bazel-io pushed a commit to bazelbuild/bazel that referenced this issue Feb 23, 2022
Implements bazelbuild/starlark#185 in Java Starlark.

Closes #14824.

PiperOrigin-RevId: 430556229
fmeum added a commit to fmeum/bazel that referenced this issue Feb 24, 2022
Implements bazelbuild/starlark#185 in Java Starlark.

Closes bazelbuild#14824.

PiperOrigin-RevId: 430556229
Wyverald pushed a commit to bazelbuild/bazel that referenced this issue Feb 24, 2022
Implements bazelbuild/starlark#185 in Java Starlark.

Closes #14824.

PiperOrigin-RevId: 430556229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants