Closed Bug 1023839 Opened 8 years ago Closed 25 days ago
Use js-indent-level in .js, .jsm emacs file variables, standardize on mode: js
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.
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.
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.
I'll push this through. Tom, is this still the .dir-locals.el file you recommend?
Assignee: nobody → jimb
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee: jimb → nobody
Status: ASSIGNED → NEW
Status: NEW → RESOLVED
Closed: 25 days ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.