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

vim: Implement named registers #12895

Merged
merged 7 commits into from
Jun 12, 2024
Merged

Conversation

p-A-uLo
Copy link
Contributor

@p-A-uLo p-A-uLo commented Jun 11, 2024

Release Notes:

  • vim: Add support for register selection "a-"z, "0-"9, "-. "_ and "% (#11511)

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jun 11, 2024
@p-A-uLo p-A-uLo marked this pull request as ready for review June 11, 2024 15:59
@maxdeviant maxdeviant changed the title [vim] Implement named registers vim: Implement named registers Jun 11, 2024
@ConradIrwin
Copy link
Collaborator

🥳 Awesome!

@p-A-uLo I think it would be best to do this in the way we do marks where we just wait for input after the " instead of hardcoding the ascii range. Was there a reason you chose to do it this way?

Happy to talk this through if you want: https://calendly.com/conradirwin/pairing

@p-A-uLo
Copy link
Contributor Author

p-A-uLo commented Jun 11, 2024

@ConradIrwin Ok, I'll look into it.

@p-A-uLo
Copy link
Contributor Author

p-A-uLo commented Jun 11, 2024

@ConradIrwin I implemented the changes you proposed. I may have done something wrong though : to avoid conflicts I rebased and force pushed. Now CI fails. Could you have a look ?

@ConradIrwin
Copy link
Collaborator

re-running....

@ConradIrwin
Copy link
Collaborator

@p-A-uLo I read though this more, and looked at the vim docs, and came to a few conclusions:

  • I think we should identify registers by char and store them in a hash map so we can support the various special vim registers as well as any characters people want to use, and we don't need to pre-allocate a mostly empty array.
  • "0 (used for yanks) is actually different from "1-"9 (used for multi-line deletes) and they are both different again from "- (who knew!)
  • "+ and "* always talk directly to the system clipboard
  • I think we can ship this without supporting all the special registers. For now I've added "%. I think "/, "#, ":, ". would be pretty easy to add, but "= is probably a won't fix for now.

I ended up refactoring as part of exploring how should work, so I've updated this branch with that.

Anything else you think we should do? Otherwise I think this is good to merge.

@0x2CA
Copy link
Contributor

0x2CA commented Jun 12, 2024

hi ,vim Command display #4447 The function is broken.

@ConradIrwin
Copy link
Collaborator

Thanks @0x2CA - added that!

@ConradIrwin ConradIrwin merged commit 001f17c into zed-industries:main Jun 12, 2024
8 checks passed
@baldwindavid
Copy link
Contributor

Amazing work, @p-A-uLo and @ConradIrwin ! This is a really big deal.

@p-A-uLo
Copy link
Contributor Author

p-A-uLo commented Jun 12, 2024

Thanks @ConradIrwin, this is making zed a quite good vim now !

@ConradIrwin
Copy link
Collaborator

thank you too! Keep the PRs coming :D.

ming900518 pushed a commit to ming900518/zed that referenced this pull request Jun 14, 2024
Release Notes:

- vim: Add support for register selection `"a`-`"z`, `"0`-`"9`, `"-`.
`"_` and `"%`
([zed-industries#11511](zed-industries#11511))

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
fallenwood pushed a commit to fallenwood/zed that referenced this pull request Jun 18, 2024
Release Notes:

- vim: Add support for register selection `"a`-`"z`, `"0`-`"9`, `"-`.
`"_` and `"%`
([zed-industries#11511](zed-industries#11511))

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
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.

None yet

4 participants