Open Bug 1023839 Opened 5 years ago Updated 9 months ago

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

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: anaran, Assigned: jimb)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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.
Attachment #8438397 - Attachment is obsolete: true
Attachment #8439793 - Flags: review?(vporof)
Attachment #8439793 - Flags: review?(paul)
Attachment #8439793 - Flags: review?(bgrinstead)
See Bug 914753 for the context of my js2-mode remark, which is lacking context here without that background.
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.
Attachment #8439793 - Flags: review?(vporof)
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?
Attachment #8439793 - Flags: review?(bgrinstead) → feedback?(nfitzgerald)
Attachment #8439793 - Flags: feedback?(nfitzgerald) → feedback+
There's no need to say mode: js in any of these files. See:
https://bugzilla.mozilla.org/show_bug.cgi?id=914753#c11
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.
Attached file .dir-locals.el
Attachment #8439793 - Flags: review?(paul)
I'll push this through. Tom, is this still the .dir-locals.el file you recommend?
Assignee: nobody → jimb
Flags: needinfo?(ttromey)
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.
Flags: needinfo?(ttromey)
Product: Firefox → DevTools
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
You need to log in before you can comment on or make changes to this bug.