tpope / vim-repeat

repeat.vim: enable repeating supported plugin maps with "."
http://www.vim.org/scripts/script.php?script_id=2136
2.59k stars 81 forks source link

Don't increment b:changedtick, offer invalidate instead. #2

Closed inkarkat closed 12 years ago

inkarkat commented 12 years ago

repeat#set() so far automatically incremented b:changedtick. Problems with this:

  1. The way that was done clobbered the expression register "=.
  2. It causes the "readonly" warning and "Cannot make changes" error in readonly/nomodifiable buffers, so mappings that don't modify anything cannot be repeated there.
  3. It's actually not needed most of the time, because many user mappings and all repeatable Vim built-in normal mode commands I know (with the exception of yank with cpo+=y) actually do modify the buffer themselves.

For the exceptional case where the user has a set of related mappings, one that repeats naturally (e.g. a custom operator, via g@), and one that invokes repeat#set(), and both do not modify the buffer, a new function repeat#invalidate() is offered. This should be called by the former mapping, and all is well.

inkarkat commented 12 years ago

Forgot to mention: 2b) Marking unmodified buffers modified, causing confusion ("why did a repeat of an immutable mapping cause a change?").

tpope commented 12 years ago

Okay, so in response to the original points:

  1. Definitely willing to address this.
  2. Is this really an issue? If I do guu on a blank line, for example, it doesn't change anything, and I still get that warning.
  3. I can't remember the edge case I was trying to address with this, so I am willing to try removing it and seeing what breaks.

Is there a real world plugin that would benefit from repeat#invalidate()? I'd rather leave it out until I see the need for it.

inkarkat commented 12 years ago

Thanks! My responses:

  1. I had first used setline(1, getline(1)) instead, but then got rid of the modification altogether.
  2. guu is a bad example, since it's (except in this edge case) a mutating command. Rather, think of an "extended yank" or "send to external command" kind of command which never modifies the buffer.
  3. I'm using repeat.vim in a lot of my personal mappings (thanks so much for repeat.vim; it's so clever and powerful that it should be part of the core Vim distribution!), and haven't seen any problems yet. The only issue I see is with the mentioned cpo+=y, but I think the benefits greatly outweigh that.

I have a real-world use for repeat#invalidate(), it's in a "repeatable accumulating yank" plugin (unpublished so far). Granted, its use is rare (that's why I added extensive documentation), but hey, the function itself is only a one-liner.

tpope commented 12 years ago

See 1118a8324fa2379777b54260dc04b4c14c69bc4a for a fix to 1.

I'm still not sure I follow with 2. I can't think of a single built-in that's repeatable with . and that doesn't trigger that warning. But I guess 2b is valid for guu.

I guess I'm willing to try it, with the caveat that I want a long bake time before I cut the next release.

tpope commented 12 years ago

Specifically, if you'd downplay the repeat#invalidate() documentation a bit (a line or two immediately before the function is fine), I'll merge it in.

Oh, and be sure to squash into one commit, and kill the trailing period in the subject line while you're at it. Thanks!

inkarkat commented 12 years ago

Alright, I've rebased and incorporated your feedback.

Specifically, if you'd downplay the repeat#invalidate() documentation a bit (a line or two immediately before the function is fine), I'll merge it in.

This is a very special corner case, so I thought a bit of verbosity would help. OK, I tried to distill the essence into two lines.

Oh, and be sure to squash into one commit, and kill the trailing period in the subject line while you're at it. Thanks!

You're taking this really serious :-)

tpope commented 12 years ago

Merged. Thanks.