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.

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

confusion with file patterns #115

Closed
rodneyrehm opened this issue Dec 30, 2016 · 2 comments · Fixed by #116
Closed

confusion with file patterns #115

rodneyrehm opened this issue Dec 30, 2016 · 2 comments · Fixed by #116

Comments

@rodneyrehm
Copy link
Contributor

My package.json looks as follows:

{
  "scripts": {
    "lint-staged": "lint-staged"
  },
  "pre-commit": "lint-staged",
  "lint-staged": {
    "*.js": "eslint",
    "src/**/*.js": "eslint"
  },
}

My directory structure looks as follows:

├── .git
├── package.json
├── alpha.js                          [should match "*.js"]
├── src
    └── bravo
        └── charlie.js                [should match "src/**/*.js"]

I'm trying to commit alpha.js and src/bravo/charlie.js and expect both patterns given in lint-staged to each match one pattern, considering everything is relative to CWD. However neither matches.

If I prefix the patterns with **/ the files are matched. That's because the paths handed to minimatch are made absolute based on gitDir but not made relative against CWD again. The test suggests this is an oversight, or the test is mislabeled.

lint-staged v3.2.4
node v7.3.0
npm v3.10.10

my linters are engaged properly, if I replace the resolvePaths.js with the following:

module.exports = function resolvePaths(filePaths, relativeTo) {
    const cwd = process.cwd()
    const base = relativeTo || cwd
    return filePaths.map(file => path.relative(cwd, path.resolve(base, file.filename)))
}
@okonet
Copy link
Collaborator

okonet commented Dec 30, 2016

This is probably a regression. Could you create a PR with the suggested change please? We should add tests for absolute and relative paths.

rodneyrehm added a commit to rodneyrehm/lint-staged that referenced this issue Dec 30, 2016
@rodneyrehm
Copy link
Contributor Author

sure, #116 contains the fix and adapted tests.

okonet pushed a commit that referenced this issue Dec 30, 2016
…116)

* fix(resolvePaths): return CWD-relative paths - closes #115
* chore(package): removing obsolete devDependency "tmp"

Fixes #115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants