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.
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
Remove ignore
option for isGitIgnored
and isGitIgnoredSync
#225
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,12 +6,15 @@ import gitIgnore from 'ignore'; | |
import slash from 'slash'; | ||
import {toPath} from './utilities.js'; | ||
|
||
const DEFAULT_IGNORE = [ | ||
'**/node_modules/**', | ||
'**/flow-typed/**', | ||
'**/coverage/**', | ||
'**/.git', | ||
]; | ||
const gitignoreGlobOptions = { | ||
ignore: [ | ||
'**/node_modules', | ||
'**/flow-typed', | ||
'**/coverage', | ||
'**/.git', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I tested, when use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Answering this question #224 (comment) |
||
], | ||
absolute: true, | ||
}; | ||
|
||
const mapGitIgnorePatternTo = base => ignore => { | ||
if (ignore.startsWith('!')) { | ||
|
@@ -58,51 +61,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, ...gitignoreGlobOptions}); | ||
|
||
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, ...gitignoreGlobOptions}); | ||
|
||
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); | ||
}; |
There was a problem hiding this comment.
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 thinkglobby
should still ignore these paths by default in the main globby methods.There was a problem hiding this comment.
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, changingglobby()
to ignore files in these directories is a new breaking change to theglobby()
function, it out scope of this PR.And if we decide to ignore them by default, but how to unignore them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what I'm talking about. Only for
.gitignore
files. It doesn't make sense to look for.gitignore
files insidenode_modules
and doing so would create a big slowdown.For reference, here's the original commit that added the ignores: ba08350
There was a problem hiding this comment.
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.