Closed
Bug 1312191
Opened 8 years ago
Closed 7 years ago
Do not hardcode INDENT_LEVEL in xbl preprocessor
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement, P5)
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
| Tracking | Status | |
|---|---|---|
| firefox58 | --- | fixed |
People
(Reporter: Fallen, Assigned: Fallen)
Details
Attachments
(1 file, 1 obsolete file)
Right now the xbl preprocessor uses INDENT_LEVEL = 2, which lints correctly for xbl files using two space xml indent and two space code indent.
This does not work for projects that expect four space indent. I've tried to convince eslint folks that they should give access to rule config in the preprocessors, but they are against this.
I see a few ways this could be fixed:
1) attached work in progress patch, using the difference between the minimum indent and the indent of the ]]> closing tag to assume indent on a per-function level. This would allow for:
<body><![CDATA[
// two space indent
if (foo) {
bar();
}
]]>
<body><![CDATA[
// four space indent
if (foo) {
bar();
}
]]>
Note I have removed the extra indent the xbl processor adds, because we eslint-disable-line these lines anyway now.
2) Use a rule with the preprocessor:
* Disable the indent rule for xbl files using a comment in the preprocessor
* Make all lines start with zero indent
* Create a custom mozilla/xbl-indent rule that checks indent just the same, but filters out messages where an indent of 4 was expected but 0 was found. Do this only for xbl files in some way
3) Base the indent on the first change in indent in the file (assuiming we have that information given an xml parser is used), then require consistent indent of xml and js code.
I am open to other ideas!
Attachment #8803611 -
Flags: feedback?(mratcliffe)
Comment on attachment 8803611 [details] [diff] [review] WiP - v1 If this works for all current XBL files then I am happy with your approach... it makes a lot of sense.
Attachment #8803611 -
Flags: feedback?(mratcliffe) → feedback+
|
||
Updated•7 years ago
|
Severity: normal → enhancement
Priority: -- → P5
| Comment hidden (mozreview-request) |
|
Assignee | |
Comment 3•7 years ago
|
||
Ok, here we go! I've run eslint on the whole tree and there was one additional case to take care of when the cdata closer was not on the same line as the closing tag, but that was just a few lines more. Here is a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=08dc7c0b57794f66dbc5376d8db91fa0b6b80cf1
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8803611 -
Attachment is obsolete: true
|
||
Comment 4•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8916424 [details] Bug 1312191 - Do not hardcode INDENT_LEVEL in xbl preprocessor. https://reviewboard.mozilla.org/r/187554/#review194498
Attachment #8916424 -
Flags: review?(mratcliffe) → review+
Pushed by mozilla@kewis.ch: https://hg.mozilla.org/integration/autoland/rev/2ad47a18adf5 Do not hardcode INDENT_LEVEL in xbl preprocessor. r=miker
|
||
Comment 6•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/2ad47a18adf5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
|
||
Updated•6 years ago
|
Product: Testing → Firefox Build System
|
||
Updated•5 years ago
|
Version: Version 3 → 3 Branch
|
|
||
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•