bug#8754: submission of vimvars
Štěpán Němec
Just one note:
> +Only the first @code{vimvars-check-lines} lines of a file are checked
> +for a vi mode line. If a file contains both Emacs local variables and
Vim also checks N last lines of a file. Actually I'd say it's (much)
more common to have a Vim mode line at the end than at the beginning.
Štěpán
P.S.: There was also this package with a similar goal last time I
looked:
http://bitbucket.org/danderson/evimodeline
Do you know about that one?
Nix
> +(defun vimvars-enable-feature (var)
> + "Emulate VIM's :set VAR for a variables that are just boolean."
Nit-pick of the day: excess 'a'.
James Youngman
James.
James Youngman
> P.S.: There was also this package with a similar goal last time I
> looked:
>
> http://bitbucket.org/danderson/evimodeline
>
> Do you know about that one?
I didn't. But looking at it, I see it's simpler than vimvars
(meaning, it's much easier to see it's doing the right kind of thing
and generally it's probably easier to maintain) but there are some
respects in which it interacts less nicely with both Emacs and VI than
vimvars:
- vimvars knows not to apply the vi mode line when a file has Emacs
local variables too (though the user can change this)
- vimvars checks for a vi modeline at both the bottom and the top of a file
- vimvars will not regex search through an arbitrarily long file
- vimvars also supports the vi modeline items makeprg, ignorecase, wrap
- vimvars supports the "no" prefixes for each relevant option (for
turning things off)
- vimvars has some support for M-x custom
- vimvars has slightly closer emulation of vi-related editors'
behaviour with "ex: set" (specifically, it's ignored at BOL).
Slightly less relevant:
- vimvars has a regression test (not included in the initial
submission because it would need to be modified to fit in with Emacs'
tests and it was only worth doing that if the original submission was
accepted).
- vimvars has (a small amount of) Texinfo documentation
James.
--
Google Ireland Ltd., Gordon House, Barrow Street, Dublin 4, Ireland
Registered in Dublin, Ireland. Registration Number: 368047
http://tinyurl.com/345mmx
Stefan Monnier
> 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?
- 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.
- 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).
-
(if file-local-variables-alist
(not vimvars-ignore-mode-line-if-local-variables-exist)
t)))
=>
(or (not file-local-variables-alist)
(not vimvars-ignore-mode-line-if-local-variables-exist))
=>
(not (and file-local-variables-alist
vimvars-ignore-mode-line-if-local-variables-exist))
- 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.
- (equal major-mode 'c-mode) => (derived-mode-p 'c-mode).
- you set compile-command globally.
- 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
James Youngman
>
> > 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
--
Stefan Monnier
>> > 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
James Youngman
work on your other comments soon.
Lars Magne Ingebrigtsen
> Targeting 24.2 seems like a good idea to me, then. Thanks. I will
> work on your other comments soon.
respinning this patch, now is probably a good time.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog http://lars.ingebrigtsen.no/
Lars Ingebrigtsen
> James Youngman <youn...@google.com> writes:
>
>> Targeting 24.2 seems like a good idea to me, then. Thanks. I will
>> work on your other comments soon.
>
> The Emacs 24.2 window opened recently, so if you're planning on
> respinning this patch, now is probably a good time.
happen. I'm closing the bug report; please reopen if needed.