evil icon indicating copy to clipboard operation
evil copied to clipboard

Setting registers is more in line with VIM

Open nnicandro opened this issue 7 years ago • 10 comments

This fixes the odd behavior of the yank register (0) and delete registers (1-9) described in #864 and makes their behavior identical to Vim. The following changes were made:

  • Add a new variable, evil-delete-kill-ring, which stores the text for the delete registers.
  • Remove evil-was-yanked-without-register
  • Add a new variable evil-is-yank-and-delete which distinguishes between commands that yank and delete text vs commands that just yank text.
  • Add the function evil-set-register-on-yank which replaces the register setting logic in the yank functions.
  • Set the yank register when setting the unnamed register (") for behavior more in line with Vim.

I tried to figure out how I could keep the evil-was-yanked-without-register variable without changing its meaning since I know removing it will break other packages, but it couldn't be done. It was also being used in a confusing way, mainly only to prevent the setting of the yank register in evil-delete.

nnicandro avatar May 10 '18 23:05 nnicandro

This looks pretty good, thanks! Regarding breakage, I've done a quick search and it seems that other than people vendoring their packages (which is totally fine), the rest seems to have copy-pasted the yank commands. They'll have to update them to make use of your code. Nothing much we can do about that.

wasamasa avatar May 17 '18 20:05 wasamasa

An update on this, mainly I've tried to get the behavior as close to Vim's as possible. I think it is essentially identical now.

  • I've renamed evil-set-register-on-yank to evil-kill-new. Essentially this function was kill-new, but also handled the behavior of registers.

  • Handle the small delete register ?- in evil-kill-new instead of evil-delete, also make its behavior more in line with Vim's. That is, do not set register ?1 if register ?- is being set. Set both registers if the motion is one of evil-special-delete-motions.

  • Add documentation describing the behavior of registers in evil-kill-new. Mention evil-kill-new in the documentation of evil-yank and evil-delete.

  • Give an initial value to evil-delete-kill-ring and fix behavior when setting one of the delete registers. The previous behavior would set the wrong register if evil-delete-kill-ring was not at its maximum size.

  • Removed the variable evil-is-yank-and-delete in favor of using this-command set to evil-delete to distinguish between a yank command or a yank followed by a delete, i.e. a delete command. We can make this distinction by simply assuming that the text passed to evil-kill-new is due to a yank command if this-command is not evil-delete and if this-command is evil-delete then the text is due to a delete command.

    • Actually it probably makes more sense to make the default be that the text is due to a delete command and make the check for evil-yank since killing text in Emacs is essentially the same as deleting text in Vim.

nnicandro avatar Sep 25 '18 21:09 nnicandro

Force pushed to squash all of the WIP commits.

nnicandro avatar Jan 12 '19 22:01 nnicandro

@wasamasa do you think we can merge this ? This is a common issue with new Evil users coming from Vim.

syl20bnr avatar Jan 15 '19 17:01 syl20bnr

@dzop Thank you for such a great PR. There is a core dump with Emacs 26.1 in Travis. Which version of Emacs are you using ?

syl20bnr avatar Jan 15 '19 17:01 syl20bnr

I think @wasamasa has withdrawn from active participation. I'll see if I can devote some time to this, and other PRs.

TheBB avatar Jan 15 '19 17:01 TheBB

I'm using

GNU Emacs 26.1 (build 2, x86_64-apple-darwin18.0.0, Carbon Version 158 AppKit 1671) of 2018-12-27

but I think the issue is related to #945, running make test works fine. I triggered another Travis build to see if it goes through this time.

nnicandro avatar Jan 15 '19 18:01 nnicandro

Friendly bump.

I've been using this change without issue for a long time now, but I think before it gets merged I should address the removal of the evil-was-yanked-without-register variable which breaks packages like lispyville and evil-collection.

Maybe it would be better to keep that variable around for compataility...

nnicandro avatar Oct 16 '19 19:10 nnicandro

@dzop @TheBB What's the status on this? It would be nice to see this get merged at some point.

Friendly bump.

I've been using this change without issue for a long time now, but I think before it gets merged I should address the removal of the evil-was-yanked-without-register variable which breaks packages like lispyville and evil-collection.

Maybe it would be better to keep that variable around for compataility...

evil-collection uses it once, in reference to evil-mc, which has evil-was-yanked-without-register as part of a list of variables of interest, but does not otherwise record or make use of it.

The author of lispyville mentioned this change as something he would keep an eye on. I wouldn't mind making a PR to lispyville once this gets merged.

leungbk avatar Jun 18 '20 18:06 leungbk