Skip to content
Merged
Show file tree
Hide file tree
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 Sep 3, 2017
6897f6b
Merge branch 'master' into editorconfig
josephfrazier Sep 6, 2017
24821d5
Add test for more specific .editorconfig glob
josephfrazier Sep 6, 2017
9a7a17a
Update config-resolution test snapshots
josephfrazier Sep 6, 2017
7d92baf
Fix lint
josephfrazier Sep 6, 2017
2a1ae88
fixup! Update config-resolution test snapshots
josephfrazier Sep 6, 2017
54a5dcb
Ignore absent EditorConfig settings
josephfrazier Sep 7, 2017
4f52651
Remove unnecessary eslint-disable comments
josephfrazier Sep 7, 2017
ad69825
Merge branch 'master' into editorconfig
josephfrazier Sep 9, 2017
bf534cd
WIP extract helper function
josephfrazier Sep 9, 2017
53e6f17
Move editorconfig parsing outside helper
josephfrazier Sep 12, 2017
4f660e6
use async editorconfig
josephfrazier Sep 12, 2017
ccd03c6
parallelize async resolveConfig
josephfrazier Sep 12, 2017
5e6ef51
Merge branch 'master' into editorconfig
josephfrazier Sep 12, 2017
a42fa75
Add caching for editorconfig
josephfrazier Sep 12, 2017
f123c64
Fix syntax for Node v4
josephfrazier Sep 12, 2017
c4bb231
Honor editorconfig even if no prettier config is found
josephfrazier Sep 12, 2017
8404ee0
Remove unnecessary braces from arrow function
josephfrazier Sep 12, 2017
85e350b
Return null from resolveConfig() when filePath isn't provided
josephfrazier Sep 12, 2017
148d3bf
rename helper function
josephfrazier Sep 12, 2017
cfdb146
Test that prettier.clearConfigCache() doesn't throw
josephfrazier Sep 13, 2017
efed301
Use mem.clear() instead of .clear() method
josephfrazier Sep 13, 2017
5c9c3d9
Put `filePath` in front of `arr[x]`
josephfrazier Sep 13, 2017
f69d367
Make variable names explicit
josephfrazier Sep 13, 2017
e422fc0
Merge branch 'master' into editorconfig
josephfrazier Sep 14, 2017
489fc24
WIP Test that .editorconfig is overridden by other configs
josephfrazier Sep 14, 2017
f859805
Merge branch 'master' into editorconfig
josephfrazier Sep 14, 2017
7c2b545
Disregard .editorconfig when `--config` is passed
josephfrazier Sep 14, 2017
e2bc881
Regenerate README TOC
josephfrazier Sep 14, 2017
a9a8b57
Add EditorConfig info to README
josephfrazier Sep 14, 2017
6efbf89
Pin dependency versions
josephfrazier Sep 14, 2017
0ff7bec
Merge branch 'master' into editorconfig
josephfrazier Oct 8, 2017
cd1e260
Use editorconfig fork with Rollup fix
josephfrazier Oct 8, 2017
59adb59
Always return a Promise from `editorconfigAsyncNoCache`
josephfrazier Oct 8, 2017
6a95dc2
Add getLoadEditorConfigFunction like getLoadFunction
josephfrazier Oct 8, 2017
99b9619
Pass opts.config to loadEditorConfig() like load()
josephfrazier Oct 8, 2017
30c619f
Extract loadOpts
josephfrazier Oct 8, 2017
48abd24
Add sync argument to resolveConfig, use in resolveConfig.sync
josephfrazier Oct 8, 2017
f07c166
Move EditorConfig stuff into own file
josephfrazier Oct 8, 2017
619ae2e
Remove unnecessary `.js` from `require()`
josephfrazier Oct 9, 2017
f5f382b
Don't expose `sync` parameter of resolveConfig
josephfrazier Oct 9, 2017
e583cc1
Simplify resolveConfig.sync definition
josephfrazier Oct 9, 2017
734d5ad
Switch back to upstream editorconfig
josephfrazier Oct 9, 2017
3fef30d
Merge branch 'master' into editorconfig
josephfrazier Oct 12, 2017
ccc3b01
Clarify variable name: editorConfigged -> editorConfigured
josephfrazier Oct 12, 2017
8776fa6
Pass path root to editorconfig
josephfrazier Oct 12, 2017
e5e8da0
Merge branch 'master' into editorconfig
josephfrazier Oct 15, 2017
1b4b1b4
mergeEditorConfig: Ensure mergeOverrides argument is defined
josephfrazier Oct 15, 2017
128e6e8
Dedupe conditional editorconfig parsing
josephfrazier Oct 15, 2017
e9c22f9
Make test .editorconfig not set config for all file extensions
josephfrazier Oct 17, 2017
6ac1ba7
Make test .prettierrc not set config for all file extensions
josephfrazier Oct 17, 2017
49a19fe
Return null from resolveConfig when no config is found
josephfrazier Oct 17, 2017
d44420c
Clarify when resolveConfig returns null
josephfrazier Oct 18, 2017
db9877c
editorconfig: test indent_style = tab overriding indent_size with tab…
josephfrazier Nov 2, 2017
a9bc09e
editorconfig: prioritize tab_width over indent_size
josephfrazier Nov 1, 2017
26f49a4
editorconfig: let indent_style force either tab_width or indent_size …
josephfrazier Nov 1, 2017
bf282f9
editorconfig: test/fix indent_size = tab
josephfrazier Nov 2, 2017
27254d4
Move editorConfigToPrettier into separate package
josephfrazier Nov 2, 2017
5526de0
Merge branch 'master' into editorconfig
josephfrazier Nov 6, 2017
664dced
Merge branch 'master' into editorconfig
josephfrazier Nov 6, 2017
47e4b62
Merge branch 'master' into josephfrazier-editorconfig
josephfrazier Nov 8, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,11 @@ For more information on how to use the CLI to locate a file, see the [CLI](cli.m
## Configuration Schema

If you'd like a JSON schema to validate your configuration, one is available here: http://json.schemastore.org/prettierrc.

## EditorConfig

If an [`.editorconfig` file](http://editorconfig.org/) is in your project, Prettier will parse it and convert its properties to the corresponding prettier configuration. This configuration will be overridden by `.prettierrc`, etc. Currently, the following EditorConfig properties are supported:

* `indent_style`
* `indent_size`/`tab_width`
* `max_line_length`
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
"cosmiconfig": "3.1.0",
"dashify": "0.2.2",
"diff": "3.2.0",
"editorconfig": "0.14.2",
"editorconfig-to-prettier": "0.0.1",
"emoji-regex": "6.5.1",
"escape-string-regexp": "1.0.5",
"esutils": "2.0.2",
Expand All @@ -37,6 +39,7 @@
"minimatch": "3.0.4",
"minimist": "1.2.0",
"parse5": "3.0.3",
"path-root": "0.1.1",
"postcss-less": "1.1.1",
"postcss-media-query-parser": "0.2.3",
"postcss-scss": "1.0.2",
Expand Down
43 changes: 43 additions & 0 deletions src/resolve-config-editorconfig.js
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
};
44 changes: 33 additions & 11 deletions src/resolve-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const minimatch = require("minimatch");
const path = require("path");
const mem = require("mem");

const resolveEditorConfig = require("./resolve-config-editorconfig");

const getExplorerMemoized = mem(opts =>
cosmiconfig("prettier", {
sync: opts.sync,
Expand All @@ -26,23 +28,43 @@ function getLoadFunction(opts) {
return getExplorerMemoized(opts).load;
}

function resolveConfig(filePath, opts) {
function _resolveConfig(filePath, opts, sync) {
opts = Object.assign({ useCache: true }, opts);
const load = getLoadFunction({ cache: !!opts.useCache, sync: false });
return load(filePath, opts.config).then(result => {
return !result ? null : mergeOverrides(result, filePath);
});
const loadOpts = { cache: !!opts.useCache, sync: !!sync };
const load = getLoadFunction(loadOpts);
const loadEditorConfig = resolveEditorConfig.getLoadFunction(loadOpts);
const arr = [load, loadEditorConfig].map(l => l(filePath, opts.config));

const unwrapAndMerge = arr => {
const result = arr[0];
const editorConfigured = arr[1];
const merged = Object.assign(
{},
editorConfigured,
mergeOverrides(Object.assign({}, result), filePath)
);

if (Object.keys(merged).length === 0) {
return null;
Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Collaborator Author

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/.editorconfig are not present, and the cases where they are but none of their configurations pertain to the file in question. Given that the goal of resolveConfig is 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

Copy link
Copy Markdown
Member

@CiGit CiGit Oct 18, 2017

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 .prettierrc to implement format only when there is a config (vscode).

  • Empty bag -> use defaults
  • null -> skip

@robwise May have implemented it this way (atom).

Anyway, if it also takes editorConfig into account, this is not what I would need anymore. Would it be possible to expose findConfigPath in that case?

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robwise May have implemented it this way (atom).

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:

prettier config format on save options

It would be nice to have a way to differentiate so we won't have this edge case.

Copy link
Copy Markdown
Collaborator Author

@josephfrazier josephfrazier Oct 18, 2017

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:

Anyway, if it also takes editorConfig into account, this is not what I would need anymore. Would it be possible to expose findConfigPath in that case?

I assume you're talking about resolveConfigFile when you say findConfigPath. Anyway, yeah, it occurred to me that taking .editorconfig into account makes it impossible for any one file path to be the correct result of resolveConfigFile. From what I can tell, resolveConfigFile is intended mainly to be used by editor integrations that repeatedly call prettier via the CLI. If this is the case, I'm hoping that either/both:

  • Developers who use these integrations will port the .editorconfig settings to .prettierrc
  • The integrations can be rewritten to use Prettier's API

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why being able to differentiate between these cases is useful, so I'll try to add some patches implementing/testing this

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.

}

return merged;
};

if (loadOpts.sync) {
return unwrapAndMerge(arr);
}

return Promise.all(arr).then(unwrapAndMerge);
}

resolveConfig.sync = (filePath, opts) => {
opts = Object.assign({ useCache: true }, opts);
const load = getLoadFunction({ cache: !!opts.useCache, sync: true });
const result = load(filePath, opts.config);
return !result ? null : mergeOverrides(result, filePath);
};
const resolveConfig = (filePath, opts) => _resolveConfig(filePath, opts, false);

resolveConfig.sync = (filePath, opts) => _resolveConfig(filePath, opts, true);

function clearCache() {
mem.clear(getExplorerMemoized);
resolveEditorConfig.clearCache();
}

function resolveConfigFile(filePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,22 @@
exports[`CLI overrides take precedence (stderr) 1`] = `""`;

exports[`CLI overrides take precedence (stdout) 1`] = `
"console.log(
"function f() {
console.log(
\\"should have tab width 8\\"
)
}
function f() {
console.log(
\\"should have space width 2\\"
)
}
function f() {
console.log(
\\"should have space width 8\\"
)
}
console.log(
\\"jest/__best-tests__/file.js should have semi\\"
);
console.log(
Expand Down Expand Up @@ -84,7 +99,16 @@ exports[`resolves configuration file with --find-config-path file (write) 1`] =
exports[`resolves configuration from external files (stderr) 1`] = `""`;

exports[`resolves configuration from external files (stdout) 1`] = `
"console.log(\\"jest/__best-tests__/file.js should have semi\\");
"function f() {
console.log(\\"should have tab width 8\\")
}
function f() {
console.log(\\"should have space width 2\\")
}
function f() {
console.log(\\"should have space width 8\\")
}
console.log(\\"jest/__best-tests__/file.js should have semi\\");
console.log(\\"jest/Component.js should not have semi\\")
console.log(\\"jest/Component.test.js should have semi\\");
function js() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,22 @@ exports[`CLI overrides take lower precedence with --config-precedence file-overr
exports[`CLI overrides take precedence with --config-precedence cli-override (stderr) 1`] = `""`;

exports[`CLI overrides take precedence with --config-precedence cli-override (stdout) 1`] = `
"console.log(
"function f() {
console.log(
\\"should have tab width 8\\"
)
}
function f() {
console.log(
\\"should have space width 2\\"
)
}
function f() {
console.log(
\\"should have space width 8\\"
)
}
console.log(
\\"jest/__best-tests__/file.js should have semi\\"
);
console.log(
Expand Down Expand Up @@ -137,7 +152,22 @@ exports[`CLI overrides take precedence with --config-precedence cli-override (wr
exports[`CLI overrides take precedence without --config-precedence (stderr) 1`] = `""`;

exports[`CLI overrides take precedence without --config-precedence (stdout) 1`] = `
"console.log(
"function f() {
console.log(
\\"should have tab width 8\\"
)
}
function f() {
console.log(
\\"should have space width 2\\"
)
}
function f() {
console.log(
\\"should have space width 8\\"
)
}
console.log(
\\"jest/__best-tests__/file.js should have semi\\"
);
console.log(
Expand Down
92 changes: 92 additions & 0 deletions tests_integration/__tests__/config-resolution.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,98 @@ test("API resolveConfig.sync with file arg and extension override", () => {
});
});

test("API resolveConfig with file arg and .editorconfig", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/file.js")
);
return prettier.resolveConfig(file).then(result => {
expect(result).toMatchObject({
useTabs: true,
tabWidth: 8,
printWidth: 100
});
});
});

test("API resolveConfig.sync with file arg and .editorconfig", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/file.js")
);
expect(prettier.resolveConfig.sync(file)).toMatchObject({
useTabs: true,
tabWidth: 8,
printWidth: 100
});
});

test("API resolveConfig with nested file arg and .editorconfig", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/lib/file.js")
);
return prettier.resolveConfig(file).then(result => {
expect(result).toMatchObject({
useTabs: false,
tabWidth: 2,
printWidth: 100
});
});
});

test("API resolveConfig.sync with nested file arg and .editorconfig", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/lib/file.js")
);
expect(prettier.resolveConfig.sync(file)).toMatchObject({
useTabs: false,
tabWidth: 2,
printWidth: 100
});
});

test("API resolveConfig with nested file arg and .editorconfig and indent_size = tab", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/lib/indent_size=tab.js")
);
return prettier.resolveConfig(file).then(result => {
expect(result).toMatchObject({
useTabs: false,
tabWidth: 8,
printWidth: 100
});
});
});

test("API resolveConfig.sync with nested file arg and .editorconfig and indent_size = tab", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/lib/indent_size=tab.js")
);
expect(prettier.resolveConfig.sync(file)).toMatchObject({
useTabs: false,
tabWidth: 8,
printWidth: 100
});
});

test("API resolveConfig with missing file arg", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/file.shouldnotexist")
);
return prettier.resolveConfig(file).then(result => {
expect(result).toBeNull();
});
});

test("API resolveConfig.sync with missing file arg", () => {
const file = path.resolve(
path.join(__dirname, "../cli/config/editorconfig/file.shouldnotexist")
);
expect(prettier.resolveConfig.sync(file)).toBeNull();
});

test("API clearConfigCache", () => {
expect(() => prettier.clearConfigCache()).not.toThrowError();
});

test("API resolveConfig.sync overrides work with absolute paths", () => {
// Absolute path
const file = path.join(__dirname, "../cli/config/filepath/subfolder/file.js");
Expand Down
5 changes: 3 additions & 2 deletions tests_integration/cli/config/.prettierrc
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
15 changes: 15 additions & 0 deletions tests_integration/cli/config/editorconfig/.editorconfig
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
3 changes: 3 additions & 0 deletions tests_integration/cli/config/editorconfig/file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function f() {
console.log("should have tab width 8");
}
3 changes: 3 additions & 0 deletions tests_integration/cli/config/editorconfig/lib/file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function f() {
console.log("should have space width 2");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function f() {
console.log("should have space width 8");
}
4 changes: 4 additions & 0 deletions tests_integration/cli/config/js/.editorconfig
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
7 changes: 7 additions & 0 deletions tests_integration/cli/config/package/.editorconfig
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
Loading