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

Remove ignore option for isGitIgnored and isGitIgnoredSync #225

Merged
merged 2 commits into from Jan 21, 2022
Merged

Remove ignore option for isGitIgnored and isGitIgnoredSync #225

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bad86bb
Remove `ignore` option for `isGitIgnored`
fisker Jan 21, 2022
40f19f9
Ignore dirs when glob `.gitignore` file
fisker Jan 21, 2022
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
60 changes: 21 additions & 39 deletions gitignore.js
View file
Open in desktop
Expand Up @@ -6,13 +6,6 @@ import gitIgnore from 'ignore';
import slash from 'slash';
import {toPath} from './utilities.js';

const DEFAULT_IGNORE = [
'**/node_modules/**',
'**/flow-typed/**',
'**/coverage/**',
'**/.git',
];
Copy link
Owner

Choose a reason for hiding this comment

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

While we're removing the ignore option for isGitIgnored, I think globby should still ignore these paths by default in the main globby methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These were ONLY used when glob .gitignore file, changing globby() to ignore files in these directories is a new breaking change to the globby() function, it out scope of this PR.

And if we decide to ignore them by default, but how to unignore them?

Copy link
Owner

Choose a reason for hiding this comment

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

These were ONLY used when glob .gitignore file

Yes, that's what I'm talking about. Only for .gitignore files. It doesn't make sense to look for .gitignore files inside node_modules and doing so would create a big slowdown.

For reference, here's the original commit that added the ignores: ba08350

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, you are right, misunderstood.


const mapGitIgnorePatternTo = base => ignore => {
if (ignore.startsWith('!')) {
return '!' + path.posix.join(base, ignore.slice(1));
Expand Down Expand Up @@ -58,51 +51,40 @@ const ensureAbsolutePathForCwd = (cwd, p) => {

const getIsIgnoredPredicate = (ignores, cwd) => p => ignores.ignores(slash(path.relative(cwd, ensureAbsolutePathForCwd(cwd, toPath(p)))));

const getFile = async (file, cwd) => {
const filePath = path.join(cwd, file);
const content = await fs.promises.readFile(filePath, 'utf8');

return {
cwd,
filePath,
content,
};
};
const getFile = async (filePath, cwd) => ({
cwd,
filePath,
content: await fs.promises.readFile(filePath, 'utf8'),
});

const getFileSync = (file, cwd) => {
const filePath = path.join(cwd, file);
const content = fs.readFileSync(filePath, 'utf8');

return {
cwd,
filePath,
content,
};
};
const getFileSync = (filePath, cwd) => ({
cwd,
filePath,
content: fs.readFileSync(filePath, 'utf8'),
});

const normalizeOptions = ({
ignore = [],
cwd = slash(process.cwd()),
} = {}) => ({ignore: [...DEFAULT_IGNORE, ...ignore], cwd: toPath(cwd)});
const normalizeOptions = (options = {}) => ({
cwd: toPath(options.cwd) || slash(process.cwd()),
});

export const isGitIgnored = async options => {
options = normalizeOptions(options);
const {cwd} = normalizeOptions(options);

const paths = await fastGlob('**/.gitignore', options);
const paths = await fastGlob('**/.gitignore', {cwd, absolute: true});

const files = await Promise.all(paths.map(file => getFile(file, options.cwd)));
const files = await Promise.all(paths.map(file => getFile(file, cwd)));
const ignores = reduceIgnore(files);

return getIsIgnoredPredicate(ignores, options.cwd);
return getIsIgnoredPredicate(ignores, cwd);
};

export const isGitIgnoredSync = options => {
options = normalizeOptions(options);
const {cwd} = normalizeOptions(options);

const paths = fastGlob.sync('**/.gitignore', options);
const paths = fastGlob.sync('**/.gitignore', {cwd, absolute: true});

const files = paths.map(file => getFileSync(file, options.cwd));
const files = paths.map(file => getFileSync(file, cwd));
const ignores = reduceIgnore(files);

return getIsIgnoredPredicate(ignores, options.cwd);
return getIsIgnoredPredicate(ignores, cwd);
};
1 change: 0 additions & 1 deletion index.d.ts
View file
Open in desktop
Expand Up @@ -57,7 +57,6 @@ export interface Options extends FastGlobOptionsWithoutCwd {

export interface GitignoreOptions {
readonly cwd?: URL | string;
readonly ignore?: readonly string[];
}

export type GlobbyFilterFunction = (path: URL | string) => boolean;
Expand Down
6 changes: 3 additions & 3 deletions index.test-d.ts
View file
Open in desktop
Expand Up @@ -125,7 +125,7 @@ expectType<GlobTask[]>(generateGlobTasksSync('*.tmp', {ignore: ['**/b.tmp']}));
expectType<boolean>(isDynamicPattern('**'));
expectType<boolean>(isDynamicPattern(['**', 'path1', 'path2']));
expectType<boolean>(isDynamicPattern(['**', 'path1', 'path2'], {extglob: false}));
expectType<boolean>(isDynamicPattern(['**'], {cwd: new URL('https://example.com')}));
expectType<boolean>(isDynamicPattern(['**'], {cwd: new URL('file:///path/to/cwd')}));
expectType<boolean>(isDynamicPattern(['**'], {cwd: __dirname}));

// IsGitIgnored
Expand All @@ -137,7 +137,7 @@ expectType<Promise<GlobbyFilterFunction>>(
);
expectType<Promise<GlobbyFilterFunction>>(
isGitIgnored({
ignore: ['**/b.tmp'],
cwd: new URL('file:///path/to/cwd'),
}),
);

Expand All @@ -150,6 +150,6 @@ expectType<GlobbyFilterFunction>(
);
expectType<GlobbyFilterFunction>(
isGitIgnoredSync({
ignore: ['**/b.tmp'],
cwd: new URL('file:///path/to/cwd'),
}),
);
4 changes: 2 additions & 2 deletions readme.md
View file
Open in desktop
Expand Up @@ -132,7 +132,7 @@ This function is backed by [`fast-glob`](https://github.com/mrmlnc/fast-glob#isd

Returns a `Promise<(path: URL | string) => boolean>` indicating whether a given path is ignored via a `.gitignore` file.

Takes `cwd?: URL | string` and `ignore?: string[]` as options. `.gitignore` files matched by the ignore config are not used for the resulting filter function.
Takes `cwd?: URL | string` as options.

```js
import {isGitIgnored} from 'globby';
Expand All @@ -146,7 +146,7 @@ console.log(isIgnored('some/file'));

Returns a `(path: URL | string) => boolean` indicating whether a given path is ignored via a `.gitignore` file.

Takes the same options as `isGitIgnored`.
Takes `cwd?: URL | string` as options.

## Globbing patterns

Expand Down
23 changes: 0 additions & 23 deletions tests/gitignore.js
View file
Open in desktop
Expand Up @@ -44,29 +44,6 @@ test('gitignore - sync', t => {
}
});

test('ignore ignored .gitignore', async t => {
const ignore = ['**/.gitignore'];

for (const cwd of getPathValues(path.join(PROJECT_ROOT, 'fixtures/gitignore'))) {
// eslint-disable-next-line no-await-in-loop
const isIgnored = await isGitIgnored({cwd, ignore});
const actual = ['foo.js', 'bar.js'].filter(file => !isIgnored(file));
const expected = ['foo.js', 'bar.js'];
t.deepEqual(actual, expected);
}
});

test('ignore ignored .gitignore - sync', t => {
const ignore = ['**/.gitignore'];

for (const cwd of getPathValues(path.join(PROJECT_ROOT, 'fixtures/gitignore'))) {
const isIgnored = isGitIgnoredSync({cwd, ignore});
const actual = ['foo.js', 'bar.js'].filter(file => !isIgnored(file));
const expected = ['foo.js', 'bar.js'];
t.deepEqual(actual, expected);
}
});

test('negative gitignore', async t => {
for (const cwd of getPathValues(path.join(PROJECT_ROOT, 'fixtures/negative'))) {
// eslint-disable-next-line no-await-in-loop
Expand Down