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

Suggestion: stripPrefix/stripSuffix functions #6265

Open
canac opened this issue Dec 13, 2024 · 9 comments · May be fixed by #6286
Open

Suggestion: stripPrefix/stripSuffix functions #6265

canac opened this issue Dec 13, 2024 · 9 comments · May be fixed by #6286
Labels

Comments

@canac
Copy link
Contributor

canac commented Dec 13, 2024

Is your feature request related to a problem? Please describe.

The Rust standard library has strip_prefix/strip_suffix functions to remove a substring from the start or end of a string if it exists, or return None/null if the string doesn't have that substring at the start/end. I find myself wanting that function often when manipulating URLs or CLI output. Would that be a good fit for inclusion in @std? Maybe under @std/text? I would be happy to submit a PR.

Describe the solution you'd like

import { stripPrefix, stripSuffix } from 'jsr:@std/text';

assert(stripPrefix('string', 'str') === 'ing');
assert(stripPrefix('string', 'abc') === null);

assert(stripSuffix('string', 'ing') === 'str');
assert(stripSuffix('string', 'abc') === null);

Describe alternatives you've considered

Just writing a helper function whenever I need it in each project. I think it would be a generally useful feature though.

@timreichen
Copy link
Contributor

timreichen commented Dec 14, 2024

I think this is a special case and is not too difficult to achieve natively.

It can be partially done with String.prototype.replace:

string.replace(/^str/, "") // Note: returns whole string if nothing is replaced
string.replace(/^ing/, "") // Note: returns whole string if nothing is replaced

Alternatively String.prototype.startsWith and String.prototype.endsWith can also be used:

const startStripped = string.startsWith("str") ? value.slice(3) : null
const endStripped = string.startsWith("ing") ? value.slice(0, -3) : null

So I think this is too trivial and should not be abstracted one-liner in std.

@canac
Copy link
Contributor Author

canac commented Dec 14, 2024

@timreichen I'm trying to argue that it's a useful function that I've found myself wanting multiple times. I'm not saying that it's not trivial to implement. I agree with you there. startsWith is also trivial to implement (const startsWith = (string, prefix) => string.slice(0, prefix.length) === prefix), but it is still useful to have it available natively. Of course, if the maintainers aren't interested in incorporating this into @std, I respect that.

@timreichen
Copy link
Contributor

timreichen commented Dec 15, 2024

@timreichen I'm trying to argue that it's a useful function that I've found myself wanting multiple times. I'm not saying that it's not trivial to implement. I agree with you there. startsWith is also trivial to implement (const startsWith = (string, prefix) => string.slice(0, prefix.length) === prefix), but it is still useful to have it available natively. Of course, if the maintainers aren't interested in incorporating this into @std, I respect that.

I see. What I meant was that if it is just one line that gets abstracted, it probably should not be in std imo. I don't know though where std draws the line of too trivial/ too short etc.

@0f-0b
Copy link
Contributor

0f-0b commented Dec 15, 2024

The two functions in @std/fs/eol can also be implemented in a single line of code: detect(text) is equivalent to text.includes("\r\n") ? "\r\n" : text.includes("\n") ? "\n" : null, and format(text, eol) is equivalent to text.replace(/\r?\n/g, eol). They are nonetheless worth inclusion in std because the use of them leads to more readable code, which is the same reason stripPrefix and stripSuffix are useful.

@kt3k kt3k added the text label Dec 19, 2024
@lionel-rowe
Copy link
Contributor

I wonder if a better/more versatile abstraction is strip(str, pattern, options?: { suffix, prefix }), where prefix/suffix can be bools or numbers for a count of occurrences to strip.

import { strip } from 'jsr:@std/text'

assertEquals(strip('string', 'str'), 'ing')
assertEquals(strip('string', 'ing'), 'str')

assertEquals(strip('string', 'ing', { suffix: true }), 'str')
assertEquals(strip('string', 'ing', { prefix: true }), 'string')

assertEquals(strip('string', 'str', { suffix: true }), 'string')
assertEquals(strip('string', 'str', { prefix: true }), 'ing')

assertEquals(strip('sstringss', 's'), 'tring')
assertEquals(strip('sstringss', 's', { prefix: 1, suffix: 1 }), 'strings')

Maybe pattern could also be a regex, which would be a handy abstraction for not having to write out the regex twice, once with ^ and once with $.

const input = '#&(*@&abc*&($#$'

// stripping non-word chars without `strip` (v1, unreadable and error-prone)
input.replaceAll(/^[^\p{L}\p{M}\p{N}]+|[^\p{L}\p{M}\p{N}]+$/gu, '')

// stripping non-word chars without `strip`
// (v2, requires assigning a variable and recreating the regex manually)

const re = /[^\p{L}\p{M}\p{N}]+/gu
input
    .replaceAll(new RegExp(`^(?:${re.source})`, re.flags), '')
    .replaceAll(new RegExp(`(?:${re.source})$`, re.flags), '')

// stripping non-word chars with `strip`
strip(input, /[^\p{L}\p{M}\p{N}]+/u)

@timreichen
Copy link
Contributor

I think stripStart and stripEnd is more web api-ish than a general strip function. If this functionality is added to std, i would be in favour of the more specific approach.

@lionel-rowe
Copy link
Contributor

I think stripStart and stripEnd is more web api-ish than a general strip function

@timreichen Maybe all 3 then: stripStart, stripEnd, and strip (both ends), by analogy with String#trim(Start|End)?. Same options interface for each, with optional count prop (defaults to Infinity). I guess the use case of wanting to strip the same pattern from both ends but with different counts is probably niche enough to ignore from an API design PoV.

@canac
Copy link
Contributor Author

canac commented Dec 20, 2024

@timreichen Good suggestion about stripStart/stripEnd to match trimStart/trimEnd and startsWith/endsWith.

@lionel-rowe The nice thing about the Rust versions is that they return None when the prefix or suffix isn't present. That's important for my use case, and implementing it that way provides more information. If the prefix or suffix is optional, you can just do stripStart(string, prefix) ?? string. Implementing it to return null can satisfy both use cases.

@lionel-rowe
Copy link
Contributor

Implementing it to return null can satisfy both use cases.

That feels very rust-ey but not very JS-ey, and also further away from the String#trim family. It's also not clear how that should work alongside the count option. What's wrong with checking result !== str?

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

Successfully merging a pull request may close this issue.

5 participants