Subject: Re: bug#8754: submission of vimvars

From: Stefan Monnier <[email-masked]>
Date: Wed, 06 Jul 2011 12:34:32 -0400

>> > Sorry, I'm attaching the patch instead of sending it inline, contrary
>> > to the contribution guidelines, but I'll include the commit message
>> > inline.  Google already has an FSF copyright assignment for Emacs;
>> > fwiw, so do I.   Feedback on the right place in the manual to document
>> > this would be particularly welcome, since I wasn't certain which the
>> > optimal spot was.
>> This looks like an interesting feature, thanks.
>> I think we should add it to the GNU ELPA, does that sound goo to you?

> I'm not opposed, but it is not my ideal choice for a small number of reasons:
> 1. It interacts a bit with Emacs' own local variables in order to
> avoid surprising behaviour, and my guess was that it may make sense to
> put the code in the same repo as the code interpreting Emacs' own
> local variables.
> 2. I wanted to make sure this was adequately documented in the same
> place as the Emacs' various VI emulations, since people wanting to use
> a VI emulation stand a high chance of finding this useful too.

The problem is that we entered feature freeze for Emacs-24.1 a week ago,
and I don't think this is high-enough priority to overrule it.  We can
reconsider it for Emacs-24.2 (at which point we may also reconsider the
way it hooks into Emacs: we could probably make it more robust by
hooking into hack-local-variables instead, tho this may currently lack
the necessary hooks).

>> - There's some redundancy between vimvars-enabled and (memq
>>  'vimvars-obey-vim-modeline find-file-hook), I think the user should
>>  only have to use one of the two.

> I had to make some assumptions about user preferences, and I guessed
> that someone might want to leave vimvars-obey-vim-modeline in
> find-file-hook and yet disable vimvars for specific projects via
> .dir-locals.el.

That makes sense (but see below).  Note that as long as vimvars-enabled
is nil (add-hook 'find-file-hook 'vimvars-obey-vim-modeline) is
harmless, so you could arrange to add the hook eagerly (e.g. default
vimvars-enabled to nil and add the hook unconditionally), so the user
only has to deal with vimvars-enabled.

>> - if vimvars-enabled stays, it should be renamed vimvars-mode and be
>>  made into a minor-mode.
>> - vimvars-enabled being buffer-local seems odd.  What was the motivation
>>  for it?

You don't need to call make-variable-buffer-local for dir-locals to work.
make-variable-buffer-local basically says "this will only ever be set
buffer-locally", which is not right, here.  Just remove it and things
will keep working fine.

        Stefan