Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix: Reworking/improving optionator configuration for --print-config #7206

Merged
merged 1 commit into from Oct 7, 2016
Merged

Fix: Reworking/improving optionator configuration for --print-config #7206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 15 additions & 18 deletions lib/cli.js
View file
Open in desktop
Expand Up @@ -137,6 +137,21 @@ const cli = {

log.info("v" + require("../package.json").version);

} else if (currentOptions.printConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

What happens if I do this:

$ eslint --print-config foo.js bar.js

In our current implementation, we get a warning. If I'm reading this code correctly, the new version would print the config for foo.js and not warn at all about bar.js, which I think is misleading. We probably want to check that files.length is zero, and if not, throw up the same warning we currently do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought about that. You are right, the behavior has changed. So I can add that check back in. (I removed a unit test for that specific behavior. I guess I hadn't considered that use case important, oops.)

I'll add a check and I'll get that test back into tests/lib/cli.js. Sound good?

if (files.length) {
log.error("The --print-config option must be used with exactly one file name.");
return 1;
} else if (text) {
log.error("The --print-config option is not available for piped-in code.");
return 1;
}

const engine = new CLIEngine(translateOptions(currentOptions));

const fileConfig = engine.getConfigForFile(currentOptions.printConfig);

log.info(JSON.stringify(fileConfig, null, " "));
return 0;
} else if (currentOptions.help || (!files.length && !text)) {

log.info(options.generateHelp());
Expand All @@ -153,24 +168,6 @@ const cli = {

const engine = new CLIEngine(translateOptions(currentOptions));

if (currentOptions.printConfig) {
if (files.length !== 1) {
log.error("The --print-config option requires a " +
"single file as positional argument.");
return 1;
}

if (text) {
log.error("The --print-config option is not available for piped-in code.");
return 1;
}

const fileConfig = engine.getConfigForFile(files[0]);

log.info(JSON.stringify(fileConfig, null, " "));
return 0;
}

const report = text ? engine.executeOnText(text, currentOptions.stdinFilename, true) : engine.executeOnFiles(files);

if (currentOptions.fix) {
Expand Down
4 changes: 2 additions & 2 deletions lib/options.js
View file
Open in desktop
Expand Up @@ -216,8 +216,8 @@ module.exports = optionator({
},
{
option: "print-config",
type: "Boolean",
description: "Print the configuration to be used"
type: "path::String",
description: "Print the configuration for the given file"
}
]
});
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -56,7 +56,7 @@
"lodash": "^4.0.0",
"mkdirp": "^0.5.0",
"natural-compare": "^1.4.0",
"optionator": "^0.8.1",
"optionator": "^0.8.2",
"path-is-inside": "^1.0.1",
"pluralize": "^1.2.1",
"progress": "^1.1.8",
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/cli.js
View file
Open in desktop
Expand Up @@ -880,7 +880,7 @@ describe("cli", function() {
assert.equal(exitCode, 0);
});

it("should require a single positional file argument", function() {
it("should error if any positional file arguments are passed", function() {
const filePath1 = getFixturePath("files", "bar.js");
const filePath2 = getFixturePath("files", "foo.js");

Expand Down
6 changes: 3 additions & 3 deletions tests/lib/options.js
View file
Open in desktop
Expand Up @@ -359,10 +359,10 @@ describe("options", function() {
});

describe("--print-config", function() {
it("should return true when passed --print-config", function() {
const currentOptions = options.parse("--print-config");
it("should return file path when passed --print-config", function() {
const currentOptions = options.parse("--print-config file.js");

assert.isTrue(currentOptions.printConfig);
assert.strictEqual(currentOptions.printConfig, "file.js");
});
});
});