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

Surround replace should provide visual feedback on select/replace #1589

Closed
eugenesvk opened this issue Jan 27, 2022 · 6 comments
Closed

Surround replace should provide visual feedback on select/replace #1589

eugenesvk opened this issue Jan 27, 2022 · 6 comments
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@eugenesvk
Copy link

Currently surround_replace operates silently and doesn't tell you that it selected / failed to select that one char around

Let's say we have (text_in_parethensis) and the cursor is in the middle

I think it might be worth adding some visual feedback in the status bar like so:

  • execute surround_replace
  • press (
    status bar output: replacing ( with ..., also would also be nice if the () are now highlighted
  • press [
    status bar output: replaced ( with [

Currently if there is no match the command is not aborted and still waits for the replacement, think that is a good idea to avoid mistakenly invoking a command bound to the replacement char, but at least it should show a warning that ( not found or something?

@eugenesvk eugenesvk added the C-enhancement Category: Improvements label Jan 27, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jan 29, 2022
@alevinval
Copy link
Contributor

If we would like to see this happen, I can work on that, it would look something like that:

Current experience
Screen.Recording.2023-07-09.at.17.09.35.mov
Proposed experience
Screen.Recording.2023-07-09.at.17.08.49.mov
  1. Add textual feedback to the status line
  2. Update selection with temporary selection to highlight the extent of the operation, and when the operation is done, restore the original selection.

@the-mikedavis
Copy link
Member

When this issue was created there was no feedback at all when doing a surround replace. (Try running the 22.03.1 release). Since then the pseudotext display in the bottom right corner has been fixed for the surround keys (#4077), so I think this issue could be closed. Showing the command progress in the statusline is too noisy IMO.

@eugenesvk
Copy link
Author

Statusline migh be too noisy, but this

also would also be nice if the () are now highlighted

isn't, and it's also matches the Helix philosophy of acting on selection, so it makes sense to select surrounded items before replacing them

(also, the bottom right corner is too far away from your focus to convey this info)

@alevinval
Copy link
Contributor

alevinval commented Jul 10, 2023

It seems reasonable to select the positions that are about to be changed, I've opened this draft PR with an implementation for it: #7588

@alevinval
Copy link
Contributor

The change has been merged, I think the main concerns on this request have been addressed and we can close the issue 👍

@the-mikedavis
Copy link
Member

Closed by #7588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

5 participants