GNU bug report logs - #79038
[PATCH] Add support for Vi/Vim modelines.

Previous Next

Full log

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: James Youngman <james <at> youngman.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79038 <at> debbugs.gnu.org, youngman <at> google.com
Subject: Re: [PATCH] Add support for Vi/Vim modelines.
Date: Sat, 19 Jul 2025 13:26:29 +0300

> From: James Youngman <james <at> youngman.org>
> Date: Thu, 17 Jul 2025 08:44:27 +0100
> Cc: James Youngman <james <at> youngman.org>, James Youngman <youngman <at> google.com>
> 
> I'd like to contribute support for Vi/Vim modelines.  These are
> somewhat similar to Emacs' local variables.  The basic idea is that
> this makes it easier to collaborate with a software author who uses Vi
> modelines to correctly set up their editor, and expects others to use
> consistent settings.
> 
> This change should be covered by the copyright assignment my employer
> (Google) already made for GNU Emacs.
> 
> This is my first Emacs patch, so I apologise for any misunderstandings
> or omissions.  The patch itself is attached, but here is the ChangeLog
> entry:
> 
>     Add support for Vi/Vim modelines.
> 
>     * lisp/emulation/vimvars.el: New file, implementing support for Vi
>       modelines.
>     * test/lisp/emulation/vimvars-tests.el: New file; tests for this
>       feature.
>     * doc/emacs/custom.texi(Vimvars): Describe this feature.
>     * doc/misc/viper.texi(Modeline): Mention this feature.
>     * etc/NEWS: Mention this change.

Thanks, please see the comments below.

> * lisp/emulation/vimvars.el: New file, implementing support for Vi
>   modelines.
> * test/lisp/emulation/vimvars-tests.el: New file; tests for this
>   feature.
> * doc/emacs/custom.texi(Vimvars): Describe this feature.
> * doc/misc/viper.texi(Modeline): Mention this feature.
> * etc/NEWS: Mention this change.

In your future revisions of the patch, please mention the bug number.

> ---
>  doc/emacs/custom.texi                |  24 ++
>  doc/misc/viper.texi                  |  16 +
>  etc/NEWS                             |   6 +
>  lisp/emulation/vimvars.el            | 222 ++++++++++++++
>  test/lisp/emulation/vimvars-tests.el | 428 +++++++++++++++++++++++++++
>  5 files changed, 696 insertions(+)
>  create mode 100644 lisp/emulation/vimvars.el
>  create mode 100644 test/lisp/emulation/vimvars-tests.el

Hmm.. I'm not sure we want to advertise this feature in custom.texi.
It's related to Vi and its emulations, so viper.texi seems like the
right place for it.

Stefan, WDYT about this?  Maybe this is better suited to be an ELPA
package?

> diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
> index 9b545b9fd0a..bfff00cc8f1 100644
> --- a/doc/emacs/custom.texi
> +++ b/doc/emacs/custom.texi
> @@ -777,6 +777,7 @@ Variables
>  * Directory Variables:: How variable values can be specified by directory.
>  * Connection Variables:: Variables which are valid for buffers with a
>                             remote default directory.
> +* Vimvars::             Support for Vi/Vim modelines.
>  @end menu

But if we do eventually decide to describe this in the Emacs user
manual, the @detailmenu menu in emacs.texi should also be updated.

> +The Vi editor (and many of its clones and implementations, including
> +Vim) supports a feature like file local variables. Emacs can optionally
> +support these, like this:                        ^^

Our convention is to leave 2 space between sentences.

> +@cindex risky variable
> +The Vim modeline setting @code{makeprg} is translated to
> +@code{compile-command}, but this is not treated as an unsafe local
> +variable.

I don't understand the purpose of the @cindex entry here with such a
general text.  I also don't understand why makeprg is not treated as a
risky variable.  can you explain?

> +This has an effect similar to adding @samp{set modeline} to
> +@file{~/.vimrc}.

I think "This" here alludes to the @lisp example, not to the text
after it.  So I think rearranging the text is in order.

> ++++
> +** Support for Vim/Vi modelines

Heading lines in NEWS should end in a period.

> diff --git a/lisp/emulation/vimvars.el b/lisp/emulation/vimvars.el
> new file mode 100644
> index 00000000000..50b56cc6912
> --- /dev/null
> +++ b/lisp/emulation/vimvars.el
> @@ -0,0 +1,222 @@
> +;;; vimvars.el --- Emacs support for Vi modelines
> +
> +;; Copyright (C) 2010,2011,2025 Free Software Foundation, Inc.
> +
> +;; Author: James Youngman <james <at> youngman.org>
> +;; Maintainer: James Youngman <james <at> youngman.org>
> +;; Keywords: local-variables, vi, vim, emulations
> +
> +;; vimvars is free software: you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +
> +;; vimvars is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with vimvars.  If not, see <http://www.gnu.org/licenses/>.

This preamble should be identical to the other files in the Emacs
source tree.

> +(defcustom vimvars-check-lines 5
> +  "The number of lines in the top or bottom of a file that we will search for Vim settings (Vim itself checks 5)."

The first line of a doc string should be a single sentence shorter
than 80 characters.

> +(defcustom vimvars-ignore-mode-line-if-local-variables-exist t
> +  "If non-nil, Vim mode lines are ignored in files that have Emacs local variables."

Likewise.

Previous Next

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