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] Easy motion #12106

Closed
wants to merge 143 commits into from
Closed

[WIP] Easy motion #12106

wants to merge 143 commits into from

Conversation

babyccino
Copy link
Contributor

@babyccino babyccino commented May 21, 2024

Requested at #4930

Release Notes:

Current state:

  • By word (word(w) and full word(W) are working)
  • Row mode is working

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label May 21, 2024
@0x2CA
Copy link
Contributor

0x2CA commented May 22, 2024

Hi, I'm looking forward to this feature, although I'm a vim user, I understand that there is a partial audience of non-vim users, can these features be made public?
#5289

@ConradIrwin ConradIrwin self-assigned this May 22, 2024
@ConradIrwin
Copy link
Member

@babyccino Thanks for sending this!

I'm curious about the overlay approach vs the updates to the HighlightStyle we explored on the call? (I haven't measured it, but I'm worried that if you have 100 of these on the screen, it'll be quite time-consuming to recompute the position of each one on every frame).

What other feedback would be helpful at this point?

@babyccino
Copy link
Contributor Author

@babyccino Thanks for sending this!

I'm curious about the overlay approach vs the updates to the HighlightStyle we explored on the call? (I haven't measured it, but I'm worried that if you have 100 of these on the screen, it'll be quite time-consuming to recompute the position of each one on every frame).

What other feedback would be helpful at this point?

I realised that the highlight style approach just wasn't going to work. Trying to take away the copy on the highlight objects was proving to be a real pain. At the moment the copy is fine because that struct is pretty small so adding a field which is not copyable is totally changing the API. You'd also have to add some way of dealing with the style going over the boundary of two chunks. I just think that api is incompatible with what I'm trying to do

I didn't really think about the performance to be honest because I just assumed adding a bunch of elements would be fine like it is in the browser 😅.

The least hacky way I could see it being done is adding another layer of chunks into the display_map (overlay_map I guess?), but it didn't seem worth it if the api isn't going to be used elsewhere. But if the performance really sucks I'd be happy to do that

Other than that I don't think any feedback will be all that helpful, you can see how rough it is at the moment and by the next time I push it'll be completely different. I'll try get it sort of usuable and then we can talk about it

@babyccino
Copy link
Contributor Author

Screen.Recording.2024-05-23.at.3.55.33.PM.mov

It's not pretty but it is sort of working!

babyccino added 3 commits May 23, 2024 17:40
- used a very hacky keybind to stop vim motions overlapping with easy
-
@babyccino
Copy link
Contributor Author

Screen.Recording.2024-05-24.at.1.42.04.AM.mov

Coming along 🙂

@ConradIrwin
Copy link
Member

We (probably) just need a cx.with_content_mask (or to re-use one of the ones that already exists in editor element rendering)

@babyccino
Copy link
Contributor Author

Okay seems to be fixed now. I'll try and get some actual real world use with it next week

@candsastle
Copy link

Is this feature planned to be available outside of vim?

@machin3io
Copy link

Spectactular work by the looks of it. Thank you so much for putting in the effort.

@babyccino
Copy link
Contributor Author

@ConradIrwin sorry haven't had much time to work on this. It's now back up to date with main. I haven't really got much real world use but it at least seems to work. Are you still interested in merging this? If so what else what we need to get done to make that happen

@ConradIrwin ConradIrwin reopened this Nov 6, 2024
@ConradIrwin
Copy link
Member

TBH I am on the fence about merging it. On the one hand it's highly requested, on the other it's a large change that I'll probably end up having to maintain.

Let me chat with some people internally and try and figure out what we want to do.

If we do merge it, the code needs a bit of polish - deleting comments and unused functions, making the diff look good; etc.

@ConradIrwin
Copy link
Member

ConradIrwin commented Nov 9, 2024

Ok, so I talked about this a bit internally, and I've decided that we're not going to merge this right now.

This is very contextual, but it really boils down to three things for me:

  • Adding a bunch of extra features to the editor is quite an onus to take on. Although I think the overlays stuff is good, it adds a maintenance burden to everything else we add there.
  • This really should be a Zed extension (though obviously that's impossible right now)
  • There is a lot of code that powers this feature that is otherwise not necessary, and I don't currently have the time to make sure that it's all good.

I realize this is probably not the outcome you hoped for, and I'd like to be clear that this is just a "no right now", not a "no never".

@ConradIrwin ConradIrwin closed this Nov 9, 2024
@machin3io
Copy link

machin3io commented Nov 9, 2024

Pretty saddened by this, but I totally respect and understand your reasoning.

Can you elaborate on why it's obviously not possible as an extension right now? And if and when that may change? Thanks!

@styx
Copy link

styx commented Nov 11, 2024

@ConradIrwin totally agree it should be a plugin but it is so pity we won't have this functionality anytime soon.
Is there any roadmap or even a rough idea when general plugins support development going to start?

@baldwindavid
Copy link
Contributor

@machin3io Zed extensions can currently only add languages, themes, and slash commands (https://zed.dev/docs/extensions/developing-extensions)

I wonder if more elaborate extensions could start their lives as extensions within the Zed codebase and eventually be extracted out when the infrastructure is in place to support them. And, if so, perhaps easymotion-ish functionality might be a good early candidate for that sort of thing. Or maybe there are some existing smaller features (that I can't think of :)) that are better suited as early candidates.

@machin3io
Copy link

@baldwindavid Thanks, wasn't aware they are so limited still

@baldwindavid baldwindavid mentioned this pull request Nov 11, 2024
1 task
@babyccino
Copy link
Contributor Author

babyccino commented Nov 11, 2024

@machin3io Zed extensions can currently only add languages, themes, and slash commands (https://zed.dev/docs/extensions/developing-extensions)

I wonder if more elaborate extensions could start their lives as extensions within the Zed codebase and eventually be extracted out when the infrastructure is in place to support them. And, if so, perhaps easymotion-ish functionality might be a good early candidate for that sort of thing. Or maybe there are some existing smaller features (that I can't think of :)) that are better suited as early candidates.

I did notice the new addons field in editor.rs when I was fixing the merge conflicts on this branch so I have hope it won't be too much longer. I would be happy to pitch in to help get a vim plugins api going. I've already put so much work into this PR so I'd be happy to put in more work so it can all actually get used, esp when so many are keen on the feature. Vim motion plugins (without edits) seem like a good candidate to expand the extensions.

@babyccino
Copy link
Contributor Author

Obviously won't matter now but I've trimmed all the nonsense out of the diff, so it's now 21 files instead of 28

@machin3io
Copy link

@machin3io Zed extensions can currently only add languages, themes, and slash commands (https://zed.dev/docs/extensions/developing-extensions)
I wonder if more elaborate extensions could start their lives as extensions within the Zed codebase and eventually be extracted out when the infrastructure is in place to support them. And, if so, perhaps easymotion-ish functionality might be a good early candidate for that sort of thing. Or maybe there are some existing smaller features (that I can't think of :)) that are better suited as early candidates.

I did notice the new addons field in editor.rs when I was fixing the merge conflicts on this branch so I have hope it won't be too much longer. I would be happy to pitch in to help get a vim plugins api going. I've already put so much work into this PR so I'd be happy to put in more work so it can all actually get used, esp when so many are keen on the feature. Vim motion plugins (without edits) seem like a good candidate to expand the extensions.

Once again, thank you so much for all your work. I hope it will see the light of the day. IMO, this feature alone would push zed to another level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants