-
Notifications
You must be signed in to change notification settings - Fork 219
Add preserve case option for buffer replace. References #165. #938
Conversation
replacePattern; | ||
const replacedText = this.editor.getTextInBufferRange(bufferRange) | ||
let replacementText = findRegex ? replacedText.replace(findRegex, replacePattern) : replacePattern; | ||
replacementText = preserveCaseOnReplace ? Util.preserveCase(replacementText, replacedText): replacementText |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
lib/project/util.coffee
Outdated
else if reference is reference.toUpperCase() | ||
text.toUpperCase() | ||
else | ||
text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a difficult feature to design. You have implemented preservation of All CAPS, and of Proper case. I would have expected lower case to be preserved as well. It might be worth thinking through a few more cases, such as strings with chars other than spaces and letters (e.g. one could imaging replacing snake_case and SNAKE_CASE at the same time, one being recognizably lower case and the other upper). camelCase might be interesting too if both the find and replace had that form, although maybe now I'm getting too fancy. I would prefer to open this discussion a little bit, though, such as with an issue for this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have support for
- Title Case
- UPPER CASE
I suggest you add
- lower case
- Sentence case
These are the main ones in Wikipedia/Letter_case, and ones that can be recovered from lower case. Then the user can set the default (what they put in the replace box) to whatever mixed case they want otherwise. I realize not supporting "Sentence case" was by design, but I would expect it to keep that case. Thanks!
spec/buffer-search-spec.js
Outdated
`); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well done
lib/project/util.coffee
Outdated
else if reference is reference.toUpperCase() | ||
text.toUpperCase() | ||
else | ||
text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have support for
- Title Case
- UPPER CASE
I suggest you add
- lower case
- Sentence case
These are the main ones in Wikipedia/Letter_case, and ones that can be recovered from lower case. Then the user can set the default (what they put in the replace box) to whatever mixed case they want otherwise. I realize not supporting "Sentence case" was by design, but I would expect it to keep that case. Thanks!
a0b5e25
to
e517d2f
Compare
Distinguish between titleize and capitalize for lower case and Sentence case support. Change them to return strict cases. (Don't keep other uppercase letters.) Add a clearer test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - very well done on the tests!
First, thanks for the code review! aaa bbb -> xxx yyy # lower case
Aaa bbb -> Xxx yyy # Sentence case
aaa Bbb -> xxX yYy # ? -> default
Aaa Bbb -> Xxx Yyy # Title Case
AAA BBB -> XXX YYY # UPPER CASE
aaA bbb -> xxX yYy # ? -> default
aaa bbB -> xxX yYy # ? -> default
aaA bbB -> xxX yYy # ? -> default Is that what you wanted? Edit: Took too long to type this comment! Thanks for the approval. |
Is there anything else that I could do to make it hit a release? |
Aaaah that's what I was looking for ! Hope to see it in the next release. |
I made a PR that supports a wider range of cases like snake case, camel case, and kebab case, using spaces, dashes, underscores, or camel casing as word boundaries during conversion. Could we close this in favor of it? #1041 Mine also adds a toggle button because there really needs to be a toggle button. Multiple other editors use a toggle for preserve case, like WebStorm and Sublime. |
Thanks @paradoxxxzero |
Description of the Change
This PR adds a
preserveCaseOnReplace
option (off by default) which keeps the replaced text case. For exemple, when replacingtest
withcase
:test Test TEST
will be replaced with:case Case CASE
.This is made by swapping the replacement right before putting it in the buffer in
buffer-search.js
. ThepreserveCase
algorithm is trivial and defined inproject/util.coffee
.This only works in buffer search. The project replace is done in atom core and should be addressed in a separate ticket.
Alternate Designs
One could have added a button for this option, but a plugin configuration was chosen since it concerns only replacing (not searching) and it might not be toggled very often.
Benefits
Huge benefits arise when replacing variables name that are in different case, for instance replacing in one pass
client
,getClient
andMAX_CLIENT
withperson
,getPerson
,MAX_PERSON
.Possible Drawbacks
This adds a boolean check for each replace, low performance impact though.
Applicable Issues
#165