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

Previous Next

Full log

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: James Youngman <james <at> youngman.org>
Cc: 79038 <at> debbugs.gnu.org
Subject: Re: bug#79038: [PATCH] Add support for Vi/Vim modelines.
Date: Sat, 19 Jul 2025 12:39:45 -0400

Regarding Eli's question about keeping it as an ELPA package, I don't
have a strong opinion either way (I'm leaning towards keeping it as
a separate package since nothing in Emacs depends on it, but it also
depends on how you/we plan to maintain it in the future).
I'd be happy to add it to GNU ELPA.

Some comments below.


        Stefan


> @@ -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

How 'bout placing it next to the doc of EditorConfig?

> +@lisp
> +(require 'vimvars)
> +(add-hook 'find-file-hook 'vimvars-obey-vim-modeline)
> +@end lisp

I'd rather make it a global `vimvars-mode`, like `editorconfig-mode`.
Especially since hooking into `find-file-hook` might not be the best way
to go about it, so using a minor mode will make it easy to change
the implementation without forcing users to update their init files.

Regarding "the best way": `editorconfig-mode` and Emacs' own
`.dir-locals.el` support uses `hack-dir-local-get-variables-functions`,
so maybe you could use that.  It's probably not the *best* way, because
it's been designed for dir-local rather than file-local vars, but the
upside is that it naturally gives it lower precedence than
file-local settings.

> --- a/doc/misc/viper.texi
> +++ b/doc/misc/viper.texi
> @@ -1610,6 +1610,7 @@ Customization
>  * Packages that Change Keymaps:: How to deal with such beasts.
>  * Viper Specials::               Special Viper commands.
>  * Vi Macros::                    How to do Vi style macros.
> +* Modeline::                     Emulation for Vi modelines.
>  @end menu
>  
>  @node Rudimentary Changes
> @@ -2993,6 +2994,21 @@ Vi Macros
>  currently defined.  To see all macros along with their definitions, type
>  @kbd{M-x viper-describe-kbd-macros}.
>  
> +@node Modeline
> +@section Emulation for Vi modelines
> +
> +You can enable limited support for Vi modelines, with or without Viper,
> +with @code{vimvars}:
> +
> +@findex vimvars-obey-vim-modeline
> +@lisp
> +(require 'vimvars)
> +(add-hook 'find-file-hook 'vimvars-obey-vim-modeline)
> +@end lisp

That seems redundant with the previous section in `custom.texi`.
Pick one place and add a reference to it from the other place?

> +;;; Code:
> +
> +;;; We say "Vim" here for consistency with Emacs documentation (which
> +;;; uses "Vi"), instead of "VIM" which is what that editor's
> +;;; documentation uses as its name.

These should use ";;" rather than ";;;" (which is reserved for headings).

> +(defcustom vimvars-enabled t
> +  "If nil, Vim mode lines will be ignored."
> +  :version "31.1"
> +  :type 'boolean
> +  :group 'vimvars)

The `:group` arg is redundant (same for all the other `defucstom`s in
the file).

> +(make-variable-buffer-local 'vimvars-enabled)

Why?

> +(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)."
> +  :version "31.1"
> +  :type 'integer
> +  :group 'vimvars)

I think the compiler should emit a warning for this too-long-line docstring.

> +(defun vimvars--accept-tag (leader tag)
> +  "Return non-nil if LEADER followed by TAG should be accepted as a modeline."
> +  (cond
> +   ((equal "vim" tag) t)
> +   ((equal "vi" tag) t)
> +   ;; Accept "ex:" only when it is not at the beginning of a line.
> +   ((equal "ex" tag) (not (equal 0 (length leader))))
> +   (t nil)))

I think you can remove this function and tighten the regexp instead.

> +  (while (re-search-forward
> +          " *\\([^= ]+\\)\\(=\\([^ :]+\\)\\)?" settings-end t)
> +    (let ((variable (vimvars--expand-option-name (match-string 1))))
> +      (if (match-string 3)
> +          (vimvars--assign variable (match-string 3))
> +        (vimvars--enable-feature variable))))

(match-end 3) or (match-beginning 3) would work as well (as a boolean
test) as (match-string 3) but without allocating a new string object.

> +(defun vimvars--set-indent (indent)
> +  "Set the amount of indentation caused by tab to INDENT in a mode-aware way."
> +  (when (equal major-mode 'c-mode) (setq c-basic-offset indent)))

Check `editorconfig-indent-size-vars`; it would be nice to share code here.
Recently the same need showed up in yet some other package, tho I can't
remember where right now.

> +(defun vimvars--expand-option-name (option)
> +  "Expand the abbreviated Vim :set variable OPTION to its full name."
> +  (let ((expansion
> +         (assoc option
> +                '(("ro" "readonly")
> +                  ("sts" "softtabstop")
> +                  ("sw" "shiftwidth")
> +                  ("ts" "tabstop")
> +                  ("tw" "textwidth")))))
> +    (if expansion (cadr expansion) option)))

You can shorten the `if` to (or (cadr expansion) option).

> +;;; Not supported:
> +;;; comments/com (comment leader), because it's not language-specific in Vim.

";;;" => ";;".  Maybe also move it into the function?

Previous Next

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