Check pattern existence before glob #7587

fisker · 2020-02-14T15:26:01Z

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

alexander-akait · 2020-02-14T15:41:43Z

src/cli/util.js

+      // `dot pattern` and `expand directories` support need handle differently
+      // for backward compatibility reason only expand `directories` like a glob pattern
+      // see https://github.com/prettier/prettier/pull/6639#issuecomment-548949954
+      isGlobPattern(pattern)


Just information: test* can be directory and can be glob 😄 But maybe it is good idea to check on directory firstly, also we can use https://github.com/mrmlnc/fast-glob#isdynamicpatternpattern-options to check on glob, I think that's why I decided that we need to do an update first

@evilebottnawi is-glob is original from eslint, we don't have fast-glob yet, we don't want both fast-glob and globby, we can switch to globby function later https://github.com/sindresorhus/globby#globbyhasmagicpatterns-options

Yes, agree 👍

I don't understand this if. Why include <dir>/**/* if <dir> looks like a pattern, but is a directory? I read #6639 (comment), but it doesn't make it any clearer to me.

The comment in the code says "for backward compatibility reason", but I checked: Prettier 1.19.1 doesn't behave like this

Run

$ npx prettier src [error] No matching files. Patterns tried: src !**/node_modules/** !./node_modules/** !**/.{git,svn,hg}/** !./.{git,svn,hg}/**

We don't expand dirname, unless it's pattern like !dir and it's a dir, we search all files in this !dir, because this is we want to do in this pr, otherwise, we still say we don't glob anything in src

If we don't expand this [foo] dir , people will still not able to format all files inside [foo] , because prettier [foo]/**/* will not glob any file, he has to prettier [foo]/1.js [foo]/2.js [foo]/3.js, but we do this, he can prettier [foo]. And we are going to support prettier foo #6128 , just not in this PR.

That's why there exists escaping syntax in globs. fast-glob uses backslashes for that, which, unfortunately, makes the Windows story messier... I'll check how ESLint handles this.

What a mess. The same command eslint "x\[foo\]/*.js" matches different files on Linux (x[foo] → foo.js) and Windows (x → [foo → ] → foo.js).

An alternative escaping syntax works though: eslint "x[[]foo]/*.js".

And we are going to support prettier foo #6128 , just not in this PR.

Let's move all the work with directories to that PR. For now, in this PR, let's just skip directories. To format files inside [foo], people have the escaping syntax.

fisker · 2020-02-15T13:39:23Z

@evilebottnawi ready for review.

src/cli/util.js

Co-Authored-By: Georgii Dolzhykov <[email protected]>

…nto check-file-before-glob

src/cli/util.js

…nto check-file-before-glob # Conflicts: # src/cli/util.js

docs/cli.md

alexander-akait

Let's add tests for ./directory/directory/directory/../../file.js, ./directory/./file.js

fisker · 2020-02-18T15:47:06Z

Let's add tests for ./directory/directory/directory/../../file.js, ./directory/./file.js

7493647

alexander-akait · 2020-02-18T16:03:28Z

Let's fix conflicts

# Conflicts: # src/cli/util.js

fisker · 2020-02-18T16:10:13Z

WIP

src/cli/util.js

alexander-akait · 2020-02-19T10:35:40Z

/cc @thorn0 I think we can merge to continue working on . and expand directories

fisker added 2 commits February 14, 2020 23:23

Check files and dirs before glob

4ef2f60

Return undefined instead of null

20f6010

fisker changed the title ~~[WIP] Check files and dirs before glob~~ [WIP] Check pattern existence before glob Feb 14, 2020

alexander-akait reviewed Feb 14, 2020

View reviewed changes

fisker added 11 commits February 14, 2020 23:43

Update snap

718826f

Ignore vcs files

c0148f3

No capture

b882ae2

Filter exists directory too

d3c4ba6

Remove concat

b4ceb66

node_modules and .svn tests

0da9bd1

Exclude any files outside cwd

dc7b1ca

Ignores files in node_modules

5e44636

Only check ignored dir when exists

8f22d12

Remove inside check

85d4294

Fix snapshots

6f83a1c

fisker changed the title ~~[WIP] Check pattern existence before glob~~ Check pattern existence before glob Feb 15, 2020

fisker marked this pull request as ready for review February 15, 2020 09:56

thorn0 reviewed Feb 16, 2020

View reviewed changes

src/cli/util.js Outdated Show resolved Hide resolved

thorn0 reviewed Feb 16, 2020

View reviewed changes

src/cli/util.js Outdated Show resolved Hide resolved

fisker and others added 3 commits February 16, 2020 22:26

Update src/cli/util.js

e950c49

Co-Authored-By: Georgii Dolzhykov <[email protected]>

Rename variable, comment

4858c17

Merge branch 'check-file-before-glob' of github.com:fisker/prettier i…

c43c0fa

…nto check-file-before-glob

thorn0 mentioned this pull request Feb 16, 2020

Blog post for 2.0 #7417

Merged

thorn0 reviewed Feb 16, 2020

View reviewed changes

src/cli/util.js Show resolved Hide resolved

fisker and others added 5 commits February 17, 2020 20:01

Exclude directories

e074ccd

add changelog

ef4a3d3

Fix logic

01261ae

edit docs

11d6f76

reuse cwd variable

50149f6

Merge branch 'check-file-before-glob' of github.com:fisker/prettier i…

4923fe9

…nto check-file-before-glob # Conflicts: # src/cli/util.js

thorn0 approved these changes Feb 17, 2020

View reviewed changes

fisker commented Feb 17, 2020

View reviewed changes

docs/cli.md Show resolved Hide resolved

thorn0 added this to the 2.0 milestone Feb 17, 2020

thorn0 mentioned this pull request Feb 18, 2020

Update globby to v11 #6776

Merged

4 tasks

alexander-akait requested changes Feb 18, 2020

View reviewed changes

fisker force-pushed the check-file-before-glob branch 2 times, most recently from f30b9dc to 4236201 Compare February 18, 2020 15:43

Relative path tests

7493647

fisker force-pushed the check-file-before-glob branch from 4236201 to 7493647 Compare February 18, 2020 15:45

alexander-akait approved these changes Feb 18, 2020

View reviewed changes

Merge branch 'next' into check-file-before-glob

f765fb2

# Conflicts: # src/cli/util.js

fisker added 2 commits February 19, 2020 00:17

Resolve conflicts on util.js

2796116

Move globbyOptions to outer

1a9d26d

thorn0 approved these changes Feb 18, 2020

View reviewed changes

thorn0 reviewed Feb 18, 2020

View reviewed changes

src/cli/util.js Show resolved Hide resolved

thorn0 merged commit 647c041 into prettier:next Feb 19, 2020

fisker deleted the check-file-before-glob branch February 19, 2020 11:11

alexander-akait mentioned this pull request Feb 19, 2020

Issue finding/writing files with brackets ([]). #6854

Closed

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label May 20, 2020

lock bot locked as resolved and limited conversation to collaborators May 20, 2020

Uh oh!

Check pattern existence before glob #7587

Check pattern existence before glob #7587

Uh oh!

Conversation

fisker commented Feb 14, 2020 • edited by thorn0 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait Feb 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisker Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

alexander-akait Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisker Feb 16, 2020

Choose a reason for hiding this comment

Uh oh!

fisker Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 16, 2020

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 16, 2020

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 16, 2020

Choose a reason for hiding this comment

Uh oh!

thorn0 Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisker commented Feb 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexander-akait left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisker commented Feb 18, 2020

Uh oh!

alexander-akait commented Feb 18, 2020

Uh oh!

fisker commented Feb 18, 2020

Uh oh!

Uh oh!

alexander-akait commented Feb 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fisker commented Feb 14, 2020 •

edited by thorn0

Loading

alexander-akait Feb 14, 2020 •

edited

Loading

thorn0 Feb 16, 2020 •

edited

Loading

fisker Feb 16, 2020 •

edited

Loading

thorn0 Feb 17, 2020 •

edited

Loading

alexander-akait left a comment •

edited

Loading