String Additions #166
String Additions #166
Conversation
String capitalization, as well as index-based insertion, replacement, and removal.
Allows for terse, index-based string to act as mustache-esque templates for interpolation. sprintf() does not satisfy this requirement.
|
LGTM |
| * If the [input] is longer than [original], the returned string will | ||
| * be longer than [original]. | ||
| */ | ||
| String replaceAt(int index, String input, String original, {bool overwrite : false}) { |
There was a problem hiding this comment.
I think this would be more flexible as replace(String s, int index, String replacement, {int length}) where length defaults to replacement.length.
replace('foo', 3, 'bar') => 'foobar'
replace('foo', 0, 'bar') => 'bar'
replace('foo', 0, 'bar', length: 0) => 'barfoo'
replace('foo', 1, 'bar', length: 2) => 'fbar'
insert and remove would be sugar for calls to replace
There was a problem hiding this comment.
I was pondering something along those lines.
|
Adding some more comments, but I forgot to ask earlier if you signed the Google CLA? https://developers.google.com/open-source/cla/individual |
|
I haven't, for personal information privacy concerns (name/number, etc). If there's any way I can avoid that I'd be happy to do so. |
|
@term1nal You can't avoid it. If it's really that big of a deal, maybe I could do it on your behalf? I have signed the CLA. |
|
Fine by me. I just need to get some spare time to make the necessary changes. I had an idea on how to do the interpolation without all of the calls to the regex matcher. (Rune by rune as suggested). Might be a bit ugly, but if it's significantly faster then ohwell. |
|
Yeah, sorry, since this project is Google copyright, there's nothing I can do to exempt you from the CLA. It's all lawyerly stuff that I don't fully understand. |
|
@justinfagnani Do you know if I were to re-do a PR from a fork of his branch, if that would be fine to use my CLA? Or does it HAVE to be him? |
|
The lawyery peoples need to add in a clause or waiver for anonymous contributions and waivng rights. I have no attachment to the code I contribute other than wanting it to remain free. It's not worth dropping my docs to the internet and forego my right to privacy. I'll just give my commit over to kaendfinger after I'm doing doing the necessary tweaks that were mentioned, he's a good chap. |
|
@kaendfinger I'm not at all sure. Let me check. @term1nal The information we collect for the CLA is not public, it just sits in a document in case the lawyers need it. There might be a way you could sign and tell me out of band your name. I still understand if you don't want to sign though. |
Removed formatStringList to add later after performance concerns are researched further. Rewrote replacement to have replace be the base function with remove and insert being syntactical sugar for calls to replace.
|
I've rewritten the replacement functions and added the others as syntactical sugar as suggested. Removed formatStringList for now to figure out the performance concerns, as well as add other features. Will recommit that later on it's own. |
|
|
||
| `capitalize('this was a triumph!') => 'This Was A Triumph!'` | ||
|
|
||
| `replace` allows you to replace a porton of a string starting from a specified index. |
There was a problem hiding this comment.
s/porton/portion/ or perhaps:
replace performs a substring replacement starting at the specified index.
| }); | ||
| }); | ||
|
|
||
| group('replace', () { |
There was a problem hiding this comment.
add tests for:
startIndex < 0: throwsstartIndex == original.length: returnsreplacementstartIndex > original.length: throwsoverwrite < 0: throwsoverwrite == 0: behaves as an insertoverwrite > original.length - startIndex: replaces everything after startIndex
|
Missed the conversation about the CLA above. Can we get a resolution on how you want to proceed with that? I think the options are pretty much submit a CLA or close the PR and if someone else wants to get these in they could open a new one. Justin, were you able to get any info on whether someone else could submit the code as-is or whether it would need to be a rewrite? |
|
Uh... let's hold off on any more changes or comments on the code in this PR until we get the CLS issue resolved. I wouldn't think that simply having someone else resubmit the PR solves anything, but I'd have to check with lawyer types for that, since it's certainly out of the normal path that we have guidance for. Unfortunately, checking in on corner cases of our policy and legal matters, while required for something like this, eats up time on our end. I think that the best approach here might be to file feature requests for each API addition and let the core team, or those who've signed the CLA reimplement from scratch. That might be quicker as waiting for word from our open source team which will probably be "no" anyway. |
|
Closing since no update in two weeks. Unfortunately without a CLA, we can't proceed further on this. |
String capitalization. Index-based insertion, replacement, and removal. Similar implementation of C# String.Format(..) to allow for terse index-based string interpolation without needing to use a literal with '$var' at the time of interpolation.