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

[WIP] View based refactoring in workspace #4381

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pathwave
Copy link

This implements a new command: global_refactor, where one inputs a search regex and a new window shows up with all the lines in different files matching the search.

One can then do edits on the lines and afterwards apply those changes to the documents.
If a file was not opened in the editor from before it will be opened and the change added as a transaction.
This means that the refactoring can be undone in their respective documents.

  • Demo:
refactor.mp4

There is still a lot of work to be done but I opened a draft pr to get some feedback:

  • Is this something you want in core?
  • What should be the default keybind for this?
  • Suggestions / Ideas / Something that should be done different?

@zjp-CN
Copy link
Contributor

zjp-CN commented Oct 20, 2022

Is this something you want in core?

Yes! I tried to find a plugin in neovim for global searched text replacement, but failed. I know some tricks to do it in neovim with built-in commands and popular plugins like telescope (via sending live grep to quickfix window).

This means that the refactoring can be undone in their respective documents.

This is the one of the key and important function in my opinion. I can share the experience in nvim:

all mds under all dirs
:args **/*.md

:argdo %s/pat/replacement/ | update
" or forbid autocmd
:silent! noautocmd argdo %s/pat/replacement/ | update 

" restore contents
:argdo undo | update

@archseer
Copy link
Member

Oh cool! This reminds me of Zed's multi-buffers: https://www.youtube.com/watch?v=egI75sLT164

@CBenoit
Copy link
Member

CBenoit commented Oct 22, 2022

Very cool! Would also address #4335 beautifully 💯

@CBenoit CBenoit added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 22, 2022
@cd-a
Copy link
Contributor

cd-a commented Jan 12, 2023

I love this! Can't wait to have this in core.

@pathwave
Copy link
Author

I'm not quite happy with using the enum to separate from a normal file-buffer.
I think a better way would be to separate different "UI elements" into their own components and also separate commands for those components, so I might do that in other PR's before continuing on this.

@pascalkuthe
Copy link
Member

pascalkuthe commented Jan 14, 2023

There are potentially more features that would benefit from custom views (like diff-mode) so there is some overlap there.
Also this PR could provide a provide a better UI (I think Zeds multi-buffers are a great reference).
I thank it would be better to abstract over View rather than Document.
To me a Document really represents a single file and a View is a display of that Document where the user can edit it.
This already applies for splits but also fits diff-mode and global refactoring.

This approach does however mean that we need commands that behave differently based upon what view is visible.
I was thinking of just having a callback called when a command is dispatched.
`AbstractView::execute_command(&mut self, name: &str, f: impl FnMut(&mut View) ;
which would allow custom views to either just forward to their sub-views or hook for commands they want to "special case" (for example scrolling/movement in diff-mode should affect both views but all other commands are treated as normal).

The approach is kind of tricky for this PR because you would have to special case selection commands and introduce a notion of a view without selection but that's it.

Ideally you should not have to touch the document or text rendering/positioning itself at all (especially because #5420 will almost fully replace the old rendering/positioning code). Simply calculate the height and offset of each sub-view upfront (and keep it fixed even if text is removed) and then use a simple row-based offset into the view and use a binary search of views to find which views to render (and which not).

Since #5420 is mostly done now I might work on such an abstraction next in my quest to get diff-mode working :D

@pathwave
Copy link
Author

Yes, one of the main issues here is that the text edit commands use the current document.
My thoughts was that in a more component based layout one could have a component "text buffer" that had all the commands for editing text in it. Then there could be a "file view" or something that used this component and added its own commands for file specific things. "Global refactor view" could then also use "text buffer" as a sub component and add its own specific things.

When more and more things get added to the editor, for example "file-tree view" it makes sense that those things are separate components with their own commands and can have separate key bindings and we can avoid squashing everything into EditorView

@nfurfaro
Copy link

Very cool to see this WIP! I was searching to see if there were any plans for something similar to ZEDs multibuffers, and here it is.

@00sapo
Copy link

00sapo commented Jul 11, 2023

Hello, just to note that before of ZED, vim-esearch was doing that. I used it a lot and is one of my preferred plugins. Maybe give a look into that for ideas :)

Hope to see this PR merged asap!

@agl-alexglopez
Copy link

agl-alexglopez commented Oct 10, 2023

@pascalkuthe Hi! I'm just a new user but I've found the cursor paradigm extremely powerful and unique so far in Helix. When you say...

This approach does however mean that we need commands that behave differently based upon what view is visible.

and that a View may lose some selection capabilities, would you lose some benefit of how flexible cursors are in your standard editing session. For example suppose we open all those buffers we want to refactor as in the video example. If we are given cursors that appear in all of those views we can refactor as expected but implicitly we would also have access to a selection removal similar to (y/n/a/q)? in many text editors. We could simply cycle through primary selection/view with ( or ) and even drop selections we don't want to change with Alt-,. This is like your standard editor prompt replace but in the reverse order; decide if you want to drop anything then perform the change for the rest of the selections. I'm gathering from this thread that might be a technical challenge so no worries if it's not possible.

@kirawi kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels May 28, 2024
@verte-zerg
Copy link

Hi! Could someone describe what needs to be done to merge this PR? Maybe some of the authors can share a vision of how the feature should look. I can try to take it on and implement it.

@noor-tg
Copy link

noor-tg commented Jul 9, 2024

Very good demo. I hope this will be merged to the core.

@noor-tg
Copy link

noor-tg commented Jul 9, 2024

@pathwave, are you alive? What happened to this PR ?

@noor-tg
Copy link

noor-tg commented Jul 9, 2024

Hi! Could someone describe what needs to be done to merge this PR? Maybe some of the authors can share a vision of how the feature should look. I can try to take it on and implement it.

You can check the code . And start new PR based on this PR.

It seems this one was abandoned .

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 S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.