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
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: anaran, Unassigned)
References
Details
Attachments
(2 files, 1 obsolete file)
124.07 KB,
patch
|
fitzgen
:
feedback+
|
Details | Diff | Splinter Review |
561 bytes,
text/x-emacs-lisp
|
Details |
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.
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
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)
Reporter | ||
Comment 3•8 years ago
|
||
See Bug 914753 for the context of my js2-mode remark, which is lacking context here without that background.
Comment 4•8 years ago
|
||
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 5•8 years ago
|
||
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)
Updated•8 years ago
|
Attachment #8439793 -
Flags: feedback?(nfitzgerald) → feedback+
Comment 6•8 years ago
|
||
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•8 years ago
|
||
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•8 years ago
|
||
Updated•7 years ago
|
Attachment #8439793 -
Flags: review?(paul)
Comment 9•5 years ago
|
||
I'll push this through. Tom, is this still the .dir-locals.el file you recommend?
Assignee: nobody → jimb
Flags: needinfo?(ttromey)
Comment 10•5 years ago
|
||
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)
Updated•4 years ago
|
Product: Firefox → DevTools
Updated•4 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 11•25 days ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: jimb → nobody
Status: ASSIGNED → NEW
I don't think we want to do this anymore (I remember we removed all the top-level vim/emacs comments)
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.
Description
•