fix for handling braces in path

[MhMadHamster]

No description provided.

[MhMadHamster]

should fix #287 and #614

[es128]

Looks like it's causing a few tests to consistently fail.

Did you try a split-string based approach as suggested in #614?

[MhMadHamster]

I took braces because of discussion in #287, I honestly did not test it on Linux, since my main windows, will try to fix tests asap.

[coveralls]

Coverage decreased (-7.6%) to 90.873% when pulling 91e2752 on MhMadHamster:master into 3d91781 on paulmillr:master.

[coveralls]

Coverage decreased (-7.6%) to 90.873% when pulling b149ec0 on MhMadHamster:master into 3d91781 on paulmillr:master.

[coveralls]

Coverage decreased (-7.6%) to 90.873% when pulling b149ec0 on MhMadHamster:master into 3d91781 on paulmillr:master.

[MhMadHamster]

closing since i dont have macos so finding the problem causing the test fall would take some time for sure

[coveralls]

Coverage increased (+0.02%) to 98.471% when pulling dd71923 on MhMadHamster:master into 3d91781 on paulmillr:master.

[MhMadHamster]

https://travis-ci.org/paulmillr/chokidar/jobs/260414604 this one looks really weird because in last build it was failing on braces test and i changed only braces test, @es128 can you maybe restard this particular job please, if you have access, will much appretiate it.

[es128]

@MhMadHamster thanks this is really good work. I got the one job in Travis passing after a few retries - this isn't unusual, the CI envs are fickle, especially for OS X.

There is one last problem to solve - the "check ignore after stating" test is failing on Windows with this patch.

https://ci.appveyor.com/project/es128/chokidar/build/387

[MhMadHamster]

@es128 tbh this test is failing for me even on the current release of chokidar, using windows 10 node 6.9.1 npm 3.10.8 😕 will look into that issue.

[es128]

@MhMadHamster oh yeah? It wasn't failing on appveyor before this patch. I'm not sure what the issue is. Sporadic failures are just noise, but when the same test fails on every version in both modes it's indicative of something needing to be adjusted.

[MhMadHamster]

@es128 yeah, I remember that it wasn't failing before, at least in my environment.

[es128]

Thanks for checking into it! ❤️

[MhMadHamster]

Well, I tracked problem down to anymatch, tho I'm not sure what's the better way to resolve it, the problem with windows is: in the failing test we have variable testDir which is getting compared in ignore function to the path of watched dirs/files, the testDir itself is created using default nodejs path module, so the path inside using backslashes, tho when the function in ignored, getting called by anymatch, it's been called 2 times once with path "as is" and once with normalized path and I'm not sure what was idea behind that, therefore when we're calling the first time with value "as is", we're correctly getting false, but the second time we're calling with normalized path i.e. with forward slashes and getting true when we're expecting false.
The easy way to fix this is to use normalized path for test itself, tho as i said i'm not sure what was the idea behind calling once with path as is and once with normalized path, ideally, I think, path should be normalized in every case where it's being used, before doing anything with it.

I don't have an idea how could possibly this test pass on windows before.

[es128]

So if you use upath.normalizeSafe(path) inside the options.ignored function in that test it'll fix it? I'd be fine with that change.

The path doesn't exactly change deliberately between ignore checks, it's just that the first time it's based on user input and the second time it's based on readdirp output and gets normalized. Separate work is going to have to be done to update glob-parent to latest and deal with the inconsistencies that result out of the windows paths. It doesn't need to be a barrier to landing this PR.

[es128]

Although we do have to question why it worked before and why this patch had an impact.

[MhMadHamster]

So if you use upath.normalizeSafe(path) inside the options.ignored function in that test it'll fix it? I'd be fine with that change.

yes both for path passed in function and testDir

Although we do have to question why it worked before and why this patch had an impact.

In fact this patch do not have impact on this test, i believe breaking changes can be found here, since then check for normalized path was added.
So if you'll run chokidar tests with anymatch 1.3.0 it'll run just fine but if you update anymatch to 1.3.2 it'll fail (on windows only)

[es128]

ooook I see, thanks for clarifying. Sorry for putting you through all this, and thank you for your diligence.

Let's just update the test to use the normalized path and land this PR.

[coveralls]

Coverage increased (+0.02%) to 98.471% when pulling 752dc81 on MhMadHamster:master into 3d91781 on paulmillr:master.

[MhMadHamster]

@es128 can you restart failing jobs please, thanks.