1023839 - Use js-indent-level in .js, .jsm emacs file variables, standardize on mode: js

Status: ASSIGNED

Description

[Adrian Aichner [:anaran]]

I thing the Emacs File Variable bits are still valuable because they also tell other humans which indentation style is being used.

Usage of c-basic-offset instead of js-indent-level won't work on a per-file basis.

js-mode ignores it, js2-mode only initializes from it when the mode is loaded.

I also like js2-mode, so I have the following in my init.el to use js2-mode, even for files specifying js-mode, still obeying the specified js-indent-level.

(defalias 'js-mode 'js2-mode)
(defvaralias 'js2-basic-offset 'js-indent-level)

See also related discussion at
https://github.com/mooz/js2-mode/issues/144

I'll attach a proposed patch shortly.

Comment 1

[Adrian Aichner [:anaran]]

Created attachment 8438397 [details] [diff] [review]
0001-Bug-1023839-Use-js-indent-level-in-.js-.jsm-emacs-fi.patch

Comment 2

[Adrian Aichner [:anaran]]

Created attachment 8439793 [details] [diff] [review]
0001-Bug-1023839-Use-js-indent-level-in-.js-.jsm-emacs-fi.patch

Fixes four files where the substitution resulted in duplication of "js-indent-level: 2; ".
I used git blame to find the most recent authors of the emacs file variable lines, based on the tip of branch fx-team.
The top three contributors touched 72 of the 186 files affected.
I have asked them for review of this patch.

Comment 3

[Adrian Aichner [:anaran]]

See Bug 914753 for the context of my js2-mode remark, which is lacking context here without that background.

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 8439793 [details] [diff] [review]
0001-Bug-1023839-Use-js-indent-level-in-.js-.jsm-emacs-fi.patch

I don't use emacs much these days, so I'm not the best person to assess if these changes are warranted or not.

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8439793 [details] [diff] [review]
0001-Bug-1023839-Use-js-indent-level-in-.js-.jsm-emacs-fi.patch

Review of attachment 8439793 [details] [diff] [review]:
-----------------------------------------------------------------

I'm in the same boat as Victor.  Maybe fitzgen has some feedback about this?

Comment 6

[Jim Blandy :jimb]

There's no need to say mode: js in any of these files. See:
https://bugzilla.mozilla.org/show_bug.cgi?id=914753#c11

Comment 7

[Tom Tromey :tromey]

These days I think it would be better to remove (nearly) all of the Emacs cookies from
the source and replace them with a single .dir-locals.el file.  The only ones that would
have to remain are where choosing the mode is appropriate -- e.g., moz.build, or maybe
.h files generally.

.dir-locals.el was added in Emacs 23, released in 2009.

I'll attach the file I'm using now.

Comment 8

[Tom Tromey :tromey]

Created attachment 8490934 [details]
.dir-locals.el

Comment 9

[Jim Blandy :jimb]

I'll push this through. Tom, is this still the .dir-locals.el file you recommend?

Comment 10

[Tom Tromey :tromey]

I'm using this now:

(
 ;; Generic settings.
 (nil .
      ;; See C-h f bug-reference-prog-mode, e.g, for using this.
      ((bug-reference-url-format . "https://bugzilla.mozilla.org/show_bug.cgi?id=%s")
       (bug-reference-bug-regexp . "\\([Bb]ug ?#?\\|[Pp]atch ?#\\|RFE ?#\\|PR [a-z-+]+/\\)\\([0-9]+\\(?:#[0-9]+\\)?\\)")))

 ;; The built-in javascript mode.
 (js-mode .
     ((indent-tabs-mode . nil)
      (js-indent-level . 2)
      (js-switch-indent-offset . 2)))

 (c++-mode .
	   ((indent-tabs-mode . nil)
	    (c-basic-offset . 2)))

 (idl-mode .
	   ((indent-tabs-mode . nil)
	    (c-basic-offset . 2)))

 (css-mode .
	   ((indent-tabs-mode . nil)
	    (css-indent-offset . 2)))

 ;; New in 26.
 (mhtml-mode .
     ((indent-tabs-mode . nil)
      (js-indent-level . 2)
      (js-switch-indent-offset . 2)
      (css-indent-offset . 2)))
)


Probably not truly correct for the whole tree.