GNU bug report logs - #8754
submission of vimvars

Previous Next

Full log

Message #26 received at 8754 <at> debbugs.gnu.org (full text, mbox):

From: James Youngman <youngman <at> google.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8754 <at> debbugs.gnu.org
Subject: Re: bug#8754: submission of vimvars
Date: Wed, 6 Jul 2011 15:22:33 +0100

On Wed, Jul 6, 2011 at 14:12, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> > 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.

> - 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.   But I admit my thinking on this wasn't that
exhaustive.

> - 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?
> - if the user has to add vimvars-obey-vim-modeline to the hook, then add
>  an autoload cookie before it so that the user does not need to
>  (require 'vimvars).

[ and some other code style changes, thanks for those, I'll incorporate them. ]


> - next time someone adds support for shiftwidth in some other modes
>  (e.g. by setting smie-basic-indent), he'll probably forget to update
>  the doc, so better keep the doc less specific to the current limits.

Good tip, thanks.

> - (equal major-mode 'c-mode)  =>  (derived-mode-p 'c-mode).
> - you set compile-command globally.

Oops.   Thanks for catching this.  It turns out that makeprg cannot be
set in vi in a modeline anyway (only in .vimrc) so that feature needs
to be removed in any case.   I'm pretty sure this is the only such
case.

> - if we add it to the GNU ELPA rather than to Emacs itself, then the doc
>  needs to be moved to the Commentary section.
>
>
>        Stefan



--
Google Ireland Ltd., Gordon House, Barrow Street, Dublin 4, Ireland
Registered in Dublin, Ireland.  Registration Number: 368047
http://tinyurl.com/345mmx

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.