-
Notifications
You must be signed in to change notification settings - Fork 193
add find and replace previous buttons #1112
Conversation
|
The buttons could be arrows or any other icon from octicons |
|
Hey I feel 'replace next' and 'replace previous' seems not very useful and dangerous options, for example, suppose I mistakenly clicked 'replace previous' arrow and didn't realize that I have done so how will I get it back. |
|
@diveshkr-code Ctrl + z will undo the replace |
|
@atom @darangi @nathansobo any feedback on this? |
Surely @UziTech but what if the user clicks 'replace previous' without realizing he has done it and he doesn't see it change as the previous occurrence was not visible on the screen. |
|
@diveshkr-code There are many things the user can do to accidentally change something, but I don't think we should remove the functionality. For example, my cat walked on my keyboard the other day and entered a bunch of random text as well as pushing the page down button so I couldn't see what was changed. That doesn't mean we should remove the ability to move down a page with the page down button. The "Replace Previous" and "Replace Next" buttons are extremely useful when renaming a variables or replacing other text only in certain places. Some of the found text you might want to not replace so you can click "Find Next" and some you will want to replace so you can click "Replace Next". |
|
@diveshkr-code This PR isn't adding any new functionality. The current "Replace" button does a replace and finds the next occurrence (the same thing the replace next button will do in this PR), and the ability to replace previous is in the command palette. This PR just adds buttons for Find/Replace Previous |
@UziTech I think you have me pretty convinced cheers. |
|
@atom @darangi @rafeca @nathansobo Who is reviewing this? |
@UziTech, this has been added to our radar, the team will get back to you soonest. Thanks for the contribution 🙇♂️ |
|
Hey @UziTech, |
That is very subjective which is why we could place this behind a setting checkbox. If you don't think Atom will allow this type of UI you should also close the issues this is PR would have fixed so people don't go there thinking this is something that will be allowed. |
|
I'm struggling to understand your rationale here @sadick254. The fact that there is, by default, no "Find previous" button is just baffling, and @UziTech has done the work to implement it, so there's no workload issue here. I can't understand the conscious decision to omit such an absolutely fundamental piece of functionality when it costs nothing. The tragedy here is that I love everything else about Atom, but this is a deal-breaker. It's like you've built a Ferrari, and then removed the "reverse" gear, telling people they need to memorize some strange code just to back up. You say "the benefits are minimal" (to which I disagree, I think the benefits are minimal, absolutely critical, functionality), but what exactly is the cost? The work is done, there's plenty of space, there are clearly a lot of people who want this functionality. Is there any actual reason to not implement this? |


Description of the Change
Add
Find PreviousandReplace Previousbuttons to viewAlternate Designs
We could actually write words on the buttons but I feel like it is intuitive with the arrows. There is also tooltips on each button that says what it does so someone could hover over the button to see what it does.
Benefits
Easily find/replace previous from view. I know shift + Find finds the previous element but it is not documented anywhere and there is no equivilant shift + Replace
Possible Drawbacks
none
Applicable Issues
closes #897
fixes #269
fixes #453
fixes #467
fixes #800