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: Add :set wrap and :set nowrap #24059

Closed
wants to merge 1 commit into from

Conversation

maxbucknell
Copy link

@maxbucknell maxbucknell commented Jan 31, 2025

Closes #21147

Hi! This is my first pull request. I found this issue from the vim channel notes, and thought it was an appropriate first contribution, to try and understand things here. I hope I did it all correctly! I didn't find any tests for soft wrapping, so I have not made or edited any.

The original issue requested that set wrap toggle the soft wrap state. That's not what Vim does, so that's not what I've done. Instead, I've added separate :set wrap and :set nowrap commands.

I tried a few different things out here, including adding separate EnableSoftWrap and DisableSoftWrap actions, and converting editor::set_soft_wrap_mode() to an action. In the end I chose an optional parameter to the existing action to be the lowest touch change here.

These are (I think) the first :set ... commands added to the Vim mode in Zed, unless I've missed something. If there is a desire for more of these (number, relativenumber, shiftwidth, et al), I would be happy to contribute those as well. And, if this is not the correct way to have added such an option, please also let me know.

Thanks,
Max.

Release Notes:

  • Added Vim :set wrap and :set nowrap to enable and disable soft_wrap

Fixes zed-industries/zed#211147
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 31, 2025
@maxdeviant maxdeviant changed the title Vim: add :set wrap and :set nowrap vim: Add :set wrap and :set nowrap Jan 31, 2025
@ConradIrwin
Copy link
Member

@maxbucknell yay! Thanks for doing this.

We're having some discussions internally about actions right now, and one of the problems that we have is that actions with parameters are much harder to use in custom keymaps. As toggle softwrap is relatively high on the list of things people configure, I think we should go a different way.

I'd be happy with either:

  • A vim::Settings action that takes a key/value.
  • Adding editor::{Enable,Disable}SoftWrap

If you're keen to add more settings, the first one seems best to me.

So we'd have something like:

struct VimSetting {
  key: String,
  value: Option<String>,
}

And then when it is handled, we do the right thing.

Let me know if you want help making these changes: https://cal.com/conradirwin/pairing.

@ConradIrwin ConradIrwin self-assigned this Feb 1, 2025
@maxbucknell
Copy link
Author

I agree that the first option is nicer.

I've taken a stab at this (and might continue to later), and have also booked a pairing session for Tuesday morning.

@maxbucknell maxbucknell closed this Feb 4, 2025
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.

Editor command 'soft wrap' should also run with 'set wrap' when VIM emulation enabled
2 participants