-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Respect EditorConfig settings #2760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
f3a6ecb
[WIP] Respect EditorConfig settings
josephfrazier 6897f6b
Merge branch 'master' into editorconfig
josephfrazier 24821d5
Add test for more specific .editorconfig glob
josephfrazier 9a7a17a
Update config-resolution test snapshots
josephfrazier 7d92baf
Fix lint
josephfrazier 2a1ae88
fixup! Update config-resolution test snapshots
josephfrazier 54a5dcb
Ignore absent EditorConfig settings
josephfrazier 4f52651
Remove unnecessary eslint-disable comments
josephfrazier ad69825
Merge branch 'master' into editorconfig
josephfrazier bf534cd
WIP extract helper function
josephfrazier 53e6f17
Move editorconfig parsing outside helper
josephfrazier 4f660e6
use async editorconfig
josephfrazier ccd03c6
parallelize async resolveConfig
josephfrazier 5e6ef51
Merge branch 'master' into editorconfig
josephfrazier a42fa75
Add caching for editorconfig
josephfrazier f123c64
Fix syntax for Node v4
josephfrazier c4bb231
Honor editorconfig even if no prettier config is found
josephfrazier 8404ee0
Remove unnecessary braces from arrow function
josephfrazier 85e350b
Return null from resolveConfig() when filePath isn't provided
josephfrazier 148d3bf
rename helper function
josephfrazier cfdb146
Test that prettier.clearConfigCache() doesn't throw
josephfrazier efed301
Use mem.clear() instead of .clear() method
josephfrazier 5c9c3d9
Put `filePath` in front of `arr[x]`
josephfrazier f69d367
Make variable names explicit
josephfrazier e422fc0
Merge branch 'master' into editorconfig
josephfrazier 489fc24
WIP Test that .editorconfig is overridden by other configs
josephfrazier f859805
Merge branch 'master' into editorconfig
josephfrazier 7c2b545
Disregard .editorconfig when `--config` is passed
josephfrazier e2bc881
Regenerate README TOC
josephfrazier a9a8b57
Add EditorConfig info to README
josephfrazier 6efbf89
Pin dependency versions
josephfrazier 0ff7bec
Merge branch 'master' into editorconfig
josephfrazier cd1e260
Use editorconfig fork with Rollup fix
josephfrazier 59adb59
Always return a Promise from `editorconfigAsyncNoCache`
josephfrazier 6a95dc2
Add getLoadEditorConfigFunction like getLoadFunction
josephfrazier 99b9619
Pass opts.config to loadEditorConfig() like load()
josephfrazier 30c619f
Extract loadOpts
josephfrazier 48abd24
Add sync argument to resolveConfig, use in resolveConfig.sync
josephfrazier f07c166
Move EditorConfig stuff into own file
josephfrazier 619ae2e
Remove unnecessary `.js` from `require()`
josephfrazier f5f382b
Don't expose `sync` parameter of resolveConfig
josephfrazier e583cc1
Simplify resolveConfig.sync definition
josephfrazier 734d5ad
Switch back to upstream editorconfig
josephfrazier 3fef30d
Merge branch 'master' into editorconfig
josephfrazier ccc3b01
Clarify variable name: editorConfigged -> editorConfigured
josephfrazier 8776fa6
Pass path root to editorconfig
josephfrazier e5e8da0
Merge branch 'master' into editorconfig
josephfrazier 1b4b1b4
mergeEditorConfig: Ensure mergeOverrides argument is defined
josephfrazier 128e6e8
Dedupe conditional editorconfig parsing
josephfrazier e9c22f9
Make test .editorconfig not set config for all file extensions
josephfrazier 6ac1ba7
Make test .prettierrc not set config for all file extensions
josephfrazier 49a19fe
Return null from resolveConfig when no config is found
josephfrazier d44420c
Clarify when resolveConfig returns null
josephfrazier db9877c
editorconfig: test indent_style = tab overriding indent_size with tab…
josephfrazier a9bc09e
editorconfig: prioritize tab_width over indent_size
josephfrazier 26f49a4
editorconfig: let indent_style force either tab_width or indent_size …
josephfrazier bf282f9
editorconfig: test/fix indent_size = tab
josephfrazier 27254d4
Move editorConfigToPrettier into separate package
josephfrazier 5526de0
Merge branch 'master' into editorconfig
josephfrazier 664dced
Merge branch 'master' into editorconfig
josephfrazier 47e4b62
Merge branch 'master' into josephfrazier-editorconfig
josephfrazier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| "use strict"; | ||
|
|
||
| const editorconfig = require("editorconfig"); | ||
| const mem = require("mem"); | ||
| const pathRoot = require("path-root"); | ||
| const editorConfigToPrettier = require("editorconfig-to-prettier"); | ||
|
|
||
| const maybeParse = (filePath, config, parse) => { | ||
| const root = filePath && pathRoot(filePath); | ||
| return filePath && !config && parse(filePath, { root }); | ||
| }; | ||
|
|
||
| const editorconfigAsyncNoCache = (filePath, config) => { | ||
| return Promise.resolve(maybeParse(filePath, config, editorconfig.parse)).then( | ||
| editorConfigToPrettier | ||
| ); | ||
| }; | ||
| const editorconfigAsyncWithCache = mem(editorconfigAsyncNoCache); | ||
|
|
||
| const editorconfigSyncNoCache = (filePath, config) => { | ||
| return editorConfigToPrettier( | ||
| maybeParse(filePath, config, editorconfig.parseSync) | ||
| ); | ||
| }; | ||
| const editorconfigSyncWithCache = mem(editorconfigSyncNoCache); | ||
|
|
||
| function getLoadFunction(opts) { | ||
| if (opts.sync) { | ||
| return opts.cache ? editorconfigSyncWithCache : editorconfigSyncNoCache; | ||
| } | ||
|
|
||
| return opts.cache ? editorconfigAsyncWithCache : editorconfigAsyncNoCache; | ||
| } | ||
|
|
||
| function clearCache() { | ||
| mem.clear(editorconfigSyncWithCache); | ||
| mem.clear(editorconfigAsyncWithCache); | ||
| } | ||
|
|
||
| module.exports = { | ||
| getLoadFunction, | ||
| clearCache | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| semi: false | ||
|
|
||
| overrides: | ||
| - files: "*.js" | ||
| options: | ||
| semi: false | ||
| - files: "*.ts" | ||
| options: | ||
| semi: true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| root = true | ||
|
|
||
| [*.js] | ||
| indent_style = tab | ||
| tab_width = 8 | ||
| indent_size = 2 # overridden by tab_width since indent_style = tab | ||
| max_line_length = 100 | ||
|
|
||
| # Indentation override for all JS under lib directory | ||
| [lib/**.js] | ||
| indent_style = space | ||
| indent_size = 2 | ||
|
|
||
| [lib/indent_size=tab.js] | ||
| indent_size = tab |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| function f() { | ||
| console.log("should have tab width 8"); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| function f() { | ||
| console.log("should have space width 2"); | ||
| } |
3 changes: 3 additions & 0 deletions
3
tests_integration/cli/config/editorconfig/lib/indent_size=tab.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| function f() { | ||
| console.log("should have space width 8"); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # This file should be overridden by prettier.config.js | ||
|
|
||
| [*] | ||
| tab_width = 1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # This file should be overridden by package.json | ||
|
|
||
| [*] | ||
| tab_width = 1 | ||
|
|
||
| [*.ts] | ||
| tab_width = 1 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we should treat 0-option as not-found, but if so, we should document it on the README as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm thinking that we shouldn't distinguish between the cases where
.prettierrc/.editorconfigare not present, and the cases where they are but none of their configurations pertain to the file in question. Given that the goal ofresolveConfigis to find the configuration for a specific file, it doesn't seem especially useful to return an empty object.I just clarified this in d44420c
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking in using this difference to test for the existence of a
.prettierrcto implement format only when there is a config (vscode).@robwise May have implemented it this way (atom).
Anyway, if it also takes
editorConfiginto account, this is not what I would need anymore. Would it be possible to exposefindConfigPathin that case?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we definitely want to keep this distinction. An empty config file is different to no config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I can't tell the difference between an empty config file and a missing one, so I have to put a special note to users that they need to have a rule in it in order for it to be picked up:
It would be nice to have a way to differentiate so we won't have this edge case.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback, everyone! I see why being able to differentiate between these cases is useful, so I'll try to add some patches implementing/testing this.
EDIT: Oh, @CiGit:
I assume you're talking about
resolveConfigFilewhen you sayfindConfigPath. Anyway, yeah, it occurred to me that taking.editorconfiginto account makes it impossible for any one file path to be the correct result ofresolveConfigFile. From what I can tell,resolveConfigFileis intended mainly to be used by editor integrations that repeatedly callprettiervia the CLI. If this is the case, I'm hoping that either/both:.editorconfigsettings to.prettierrcThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this a bit more, and I'd actually like to let it be part of a separate PR later, since it's a pre-existing issue. That way, this PR can focus on the editorconfig functionality.