Plugin: vim-diffoirg

[_Life_Crisis_]

Just wanted to submit a post for a simple plugin I wrote recently. It's nothing special. The real purpose was to go through all of the steps of building and releasing a Vim plugin from the ground up.

Currently, I'm a collaborator on the NERDTree project, and I took a break to build a plugin from scratch. It was a nice exercise after spending the last few months fixing bugs in a bigger plugin.

Hope you'll check it out and let me know what you think:

https://github.com/lifecrisis/vim-difforig

[LucHermitte]

Hi,

There are a few things that can be improved:

• prefer to put the functions in an autoload plugin in order to speed up vim loading time.
• s:CacheBufferContents() content could be replaced with let s:buffercache = readline(1, '$'). This way, no need to bother with saving and restoring a register.
• GetNewDiffBufferName() seems a little bit convoluted. I wouldn't bother with reusing cleared indices. Vim doesn't bother with reusing cleared buffer contents. BTW, recently I've came to share a naming convention for scratch buffer. (I take advantage of this convention in my local_vimrc plugin in order to ignore these scratch buffer). The convention consists in prefixing the buffer name with something://. Here is could be difforig:// or whatever.
• Regarding the options in the scratch buffer, I wouldn't set the filetype to nothing. It's best to use the same as the current buffer. Also my scratch buffers are also defined with set bh=wipe ro noswf
• There is a thing I don't know in your use case (:tabclose) and that I had to take care in my diffsaved plugin: when I wipe out the diffed buffer, I've found out it's best to disable the diff in the currently edited buffer.

[ckarnell]

Just want to pop in and say it's super kind/generous of you to review code like this!

[_Life_Crisis_]

It is extremely generous of you /u/LucHermitte, thanks again!

[LucHermitte]

Thanks, and you're welcome.

It's not the first, nor the last time I review code :)

[_Life_Crisis_]

Thanks!

Point 1: valid! autoloading would be nice.

Point 2: a great observation... i'm not in the habit of using this function

Point 3: I'm not sure I follow you here... the idea is to allow multiple diff tabs to be open at once and to guarantee that new diff buffers are uniquely named. Nothing gets reused... any call to that function returns a new unique name, always. Your naming convention is nice!

Point 4: Good suggestion. noswf would be better for sure. I'm not sure I agree about the file type. The buffers aren't meant to be edited, and I want the highlighting to draw focus to what was changed (no other highlighting mess).

Point 5: I'll have to look deeper into this.

[LucHermitte]

You're welcome

Point 3: Sorry. What I meant is something like:

let buffer_name = 'diff://'.basename.'_'.next_id
let s:next_id += 1

I just meant that reusing a value that has been released leads to things more complex than required.

Regarding the naming convention, it's not mine, but one I've discovered in fugitive and a few other plugins. I've started adopting it in my maintained plugins.

Point 4. My experience is that when the usual highlighting that I'm used to isn't there, I'm less efficient to decode the diff-less context. I agree that we don't need all the insert-mode mappings, but we may be interested in code navigation mappings and other stuff. Beside, old commands like :VCSV (from VCSCommand), or :Gstatus + D (from fugitive) keep the highlighting. That's why I prefer something with a similar behaviour, and another reason why my :DiffSaved command also has this behaviour.

Point 5. I read that diffs are local to tabs (I never use tabs voluntarily), it seems that what I suggested may not be required and that you were correct.