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

CLI: Fix failure on dir with trailing slash #11000

Merged
merged 6 commits into from Jun 11, 2021
Merged

CLI: Fix failure on dir with trailing slash #11000

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85f7b2c
CLI: Fix failure on dir with trailing slash
fisker Jun 1, 2021
b37bdb7
Changelog
fisker Jun 1, 2021
d02f44e
More slashes
fisker Jun 1, 2021
3a10600
Only remove trailing slashes
fisker Jun 4, 2021
da46196
Only remove `/`, after `\` has been replaced
fisker Jun 4, 2021
b009878
Restore to use ``
fisker Jun 4, 2021
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
17 changes: 17 additions & 0 deletions changelog_unreleased/cli/11000.md
View file
Open in desktop
@@ -0,0 +1,17 @@
#### Fix failure on dir with trailing slash (#11000 by @fisker)

<!-- prettier-ignore -->
```console
$ ls
1.js 1.unknown

# Prettier stable
$ prettier . -l
1.js
$ prettier ./ -l
[error] No supported files were found in the directory: "./".

# Prettier main
$ prettier ./ -l
1.js
```
3 changes: 2 additions & 1 deletion src/cli/expand-patterns.js
View file
Open in desktop
Expand Up @@ -76,10 +76,11 @@ async function* expandPatternsInternal(context) {
input: pattern,
});
} else if (stat.isDirectory()) {
const relativePath = path.relative(cwd, absolutePath) || ".";
Copy link
Member

Choose a reason for hiding this comment

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

If the goal of this line is only to remove trailing slashes, why not do .replace(/[/\\]+$/, '') here to express the intent more clearly?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I think the middle one foo//bar was effected too.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I'll confirm when I back to laptop, if it's true, I'll add comment.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Tested, the tailing slash is the only problem, applied your suggestion.

Copy link
Sponsor Member Author

@fisker fisker Jun 4, 2021 •
edited

Choose a reason for hiding this comment

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

Your solution can't work, because \ can be the dirname, see this log https://github.com/prettier/prettier/runs/2746762536#step:6:1251

This commit da46196 works, but feel better/safer to use path.relative, added comments b009878

entries.push({
type: "dir",
glob:
escapePathForGlob(fixWindowsSlashes(pattern)) +
escapePathForGlob(fixWindowsSlashes(relativePath)) +
"/" +
getSupportedFilesGlob(),
input: pattern,
Expand Down
101 changes: 101 additions & 0 deletions tests/integration/__tests__/__snapshots__/patterns-dirs.js.snap
View file
Open in desktop
Expand Up @@ -149,6 +149,107 @@ exports[`Negative patterns with explicit files: prettier dir1/a1.js dir2/a2.js '
"
`;

exports[`Trailing slash 1: prettier ./ (stdout) 1`] = `
"!dir/a.js
dir1/a1.js
dir1/b1.js
dir1/nested1/an1.css
dir1/nested1/an1.js
dir1/nested1/bn1.js
dir2/a2.js
dir2/b2.js
dir2/nested2/an2.js
dir2/nested2/bn2.js
"
`;

exports[`Trailing slash 2: prettier .// (stdout) 1`] = `
"!dir/a.js
dir1/a1.js
dir1/b1.js
dir1/nested1/an1.css
dir1/nested1/an1.js
dir1/nested1/bn1.js
dir2/a2.js
dir2/b2.js
dir2/nested2/an2.js
dir2/nested2/bn2.js
"
`;

exports[`Trailing slash 3: prettier dir1/ (stdout) 1`] = `
"dir1/a1.js
dir1/b1.js
dir1/nested1/an1.css
dir1/nested1/an1.js
dir1/nested1/bn1.js
"
`;

exports[`Trailing slash 4: prettier dir1// (stdout) 1`] = `
"dir1/a1.js
dir1/b1.js
dir1/nested1/an1.css
dir1/nested1/an1.js
dir1/nested1/bn1.js
"
`;

exports[`Trailing slash 5: prettier .//dir2/..//./dir1// (stdout) 1`] = `
"dir1/a1.js
dir1/b1.js
dir1/nested1/an1.css
dir1/nested1/an1.js
dir1/nested1/bn1.js
"
`;

exports[`Trailing slash run in sub dir 1: prettier .. (stdout) 1`] = `
"../!dir/a.js
../dir1/a1.js
../dir1/b1.js
../dir1/nested1/an1.css
../dir1/nested1/an1.js
../dir1/nested1/bn1.js
a2.js
b2.js
nested2/an2.js
nested2/bn2.js
"
`;

exports[`Trailing slash run in sub dir 2: prettier ../ (stdout) 1`] = `
"../!dir/a.js
../dir1/a1.js
../dir1/b1.js
../dir1/nested1/an1.css
../dir1/nested1/an1.js
../dir1/nested1/bn1.js
a2.js
b2.js
nested2/an2.js
nested2/bn2.js
"
`;

exports[`Trailing slash run in sub dir 3: prettier ../dir1 (stdout) 1`] = `
"../dir1/a1.js
../dir1/b1.js
../dir1/nested1/an1.css
../dir1/nested1/an1.js
../dir1/nested1/bn1.js
"
`;

exports[`Trailing slash run in sub dir 4: prettier ../dir1/ (stdout) 1`] = `
"../dir1/a1.js
../dir1/b1.js
../dir1/nested1/an1.css
../dir1/nested1/an1.js
../dir1/nested1/bn1.js
"
`;

exports[`plugins \`*\` (stderr) 1`] = `
"[error] No parser could be inferred for file: unknown.unknown
"
Expand Down
41 changes: 39 additions & 2 deletions tests/integration/__tests__/patterns-dirs.js
View file
Open in desktop
Expand Up @@ -44,6 +44,38 @@ testPatterns("3", ["nonexistent-dir", "dir2/**/*"], { status: 2 });

testPatterns("4", [".", "dir2/**/*"], { status: 1 });

describe("Trailing slash", () => {
testPatterns("1", ["./"], { status: 1, stderr: "" });
testPatterns("2", [".//"], { status: 1, stderr: "" });
testPatterns("3", ["dir1/"], { status: 1, stderr: "" });
testPatterns("4", ["dir1//"], { status: 1, stderr: "" });
testPatterns("5", [".//dir2/..//./dir1//"], { status: 1, stderr: "" });
testPatterns(
"run in sub dir 1",
[".."],
{ status: 1, stderr: "" },
"cli/patterns-dirs/dir2"
);
testPatterns(
"run in sub dir 2",
["../"],
{ status: 1, stderr: "" },
"cli/patterns-dirs/dir2"
);
testPatterns(
"run in sub dir 3",
["../dir1"],
{ status: 1, stderr: "" },
"cli/patterns-dirs/dir2"
);
testPatterns(
"run in sub dir 4",
["../dir1/"],
{ status: 1, stderr: "" },
"cli/patterns-dirs/dir2"
);
});

describe("Negative patterns", () => {
testPatterns("1", ["dir1", "!dir1/nested1"]);
testPatterns("1a", ["dir1", "!dir1/nested1/*"]);
Expand Down Expand Up @@ -119,7 +151,12 @@ if (path.sep === "/") {
});
}

function testPatterns(namePrefix, cliArgs, expected = {}) {
function testPatterns(
namePrefix,
cliArgs,
expected = {},
cwd = "cli/patterns-dirs"
) {
const testName =
(namePrefix ? namePrefix + ": " : "") +
"prettier " +
Expand All @@ -128,7 +165,7 @@ function testPatterns(namePrefix, cliArgs, expected = {}) {
.join(" ");

describe(testName, () => {
runPrettier("cli/patterns-dirs", [...cliArgs, "-l"]).test({
runPrettier(cwd, [...cliArgs, "-l"]).test({
write: [],
...(!("status" in expected) && { stderr: "", status: 1 }),
...expected,
Expand Down