Fix: ignored-paths for Windows path (fixes #10687)

[mysticatea]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix: fixes #10687

What changes did you make? (Give an overview)

This PR fixes some problems of ignored-paths on Windows.

1. Fixes that the Windows path separator is mixed into patterns.
2. Fixes that it doesn't handle CRLF line separators correctly in .eslintignore files.

Is there anything you'd like reviewers to focus on?

I'm not sure why Appvoyer didn't find this...

[mysticatea]

I found relativePathToOldBaseDir can be ../../../foo/bar, is this a problem?

[not-an-aardvark]

Hmm, it might be. I had assumed that one of this.ignoreFileDir and this.options.cwd would be the parent of the other. If this.ignoreFileDir and this.options.cwd are in different directories, maybe this.getBaseDir() should return a common ancestor.

[not-an-aardvark]

Could this be:

const relativePathPrefix = relativePathToOldBaseDir.replace(
    new RegExp(`\\${path.sep}`, "g"),
    "/"
);

That would avoid the need to depend on process.

[mysticatea]

I updated this PR.

[platinumazure]

With some of the logic we're doing to handle filesystem-specific quirks, should we add debug() calls for some of the transformations (showing before vs after)?

Otherwise LGTM.

[mysticatea]

I added some debug()s.

[platinumazure]

LGTM, thanks!

[not-an-aardvark]

I left a couple more comments, but this mostly looks good.

[not-an-aardvark]

Would it work to split on path.sep rather than iterating over each character?

const cwd = path.resolve(this.options.cwd);
const ignoreFileDir = path.resolve(this.ignoreFileDir);

if (cwd === ignoreFileDir) {
    this._baseDir = cwd;
} else {
    const cwdParts = cwd.split(path.sep);
    const ignoreFileDirParts = ignoreFileDir.split(path.sep);

    const longerParts = cwdParts.length > ignoreFileDirParts.length
        ? cwdParts
        : ignoreFileDirParts;

    this._baseDir = longerParts.slice(
        0,
        longerParts.findIndex((_, index) => cwdParts[index] !== ignoreFileDirParts[index])
    );
}

[mysticatea]

Hm. I wrote that code as being careful to use only one traverse on the path strings. Indeed, though it's O(n) as is...

[not-an-aardvark]

Can you add tests for an ignore file with \r\n? (Git would convert the linebreaks to \n when the repository is cloned, so it would probably be necessary to create the file at runtime.)

[mysticatea]

I added it.

[not-an-aardvark]

Can you add tests for ignoredPaths.contains when ignoreFilePath and cwd are siblings? It would be useful to make sure that the default ignore patterns (e.g. node_modules) and custom ignore patterns both work.

[mysticatea]

Probably I added it.

[not-an-aardvark]

Is it feasible to add a test for when the baseDir is only a windows drive letter?

[mysticatea]

The base directory was only a Windows drive letter on Appvoyer environments.
But I'm not sure if we can write such a test safely or not, because of permissions. On my PC, the base directory has often became the home directory of my login user because the temporary directory for tests is in ~\AppData\Local\Temp.

[not-an-aardvark]

LGTM, thanks!