Allow .jshintrc and .jshintignore paths to be passed to jshint.
Ref T2039.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
T5085: Lint config is not relative to repository root - Commits
- rARC8cd9cb104798: Add some `.arclint` configuration options for `ArcanistJSHintLinter`.
Added the jshintrc and jshintignore keys to an .arclint file that was configured to use ArcanistJSHintLinter. Ran arc lint --trace and inspected the flags that were passed to jshint.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- jshint
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 520 Build 520: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
| src/lint/linter/ArcanistJSHintLinter.php | ||
|---|---|---|
| 94 | Don't we eventually use %Ls to print this? So special characters in these flags would be double-escaped, becuase we're using csprintf() here and then %Ls again later? | |
| 116–122 | Maybe we should make a convenience method for this, since several linters either do something similar or should do something similar. | |
| 144 | As above. | |
| src/lint/linter/ArcanistJSHintLinter.php | ||
|---|---|---|
| 94 | I think that you are correct, but I'll double check. | |
| 116–122 | I guess there are two possibilities here:
The former is probably better, but the latter is simpler. | |
| src/lint/linter/ArcanistJSHintLinter.php | ||
|---|---|---|
| 116–122 | I actually think (2) isn't too bad, as long as we can solve the CWD problem with setCWD($project_root) on the Future. Hopefully most/all linters raise sensible errors when you pass bad flags to them. (They also often exit with a nonzero status, but usually we can figure out if that means "error" or not.) | |
Comment Actions
Don't use csprintf.
@epriestley, you were correct about the double escaping:
- Without csprintf: 'jshint' '--reporter=/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/reporter.js' '--config=/path/to/.jshintrc' '--exclude-path=/path/to/.jshintignore' -
- With csprintf: 'jshint' '--reporter='\''/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/reporter.js'\''' '--config='\''/path/to/.jshintrc'\''' '--exclude-path='\''/path/to/.jshintignore'\''' -
Comment Actions
Don't check if the .jshintrc and .jshintignore paths exist (let jshint determine this).
Comment Actions
Closed by commit rARC8cd9cb104798 (authored by @joshuaspence, committed by @epriestley).
| src/lint/linter/ArcanistJSHintLinter.php | ||
|---|---|---|
| 113 | It's useless because file passed via STDIN. | |