sk- / git-lint

improving source code one step at a time
Apache License 2.0
237 stars 78 forks source link

git lint not playing nice w eslint/project setup #141

Closed MareoRaft closed 5 years ago

MareoRaft commented 5 years ago

The root of my project (which contains .gitlint.yaml) has a folder called client. That folder is a React project which has it's own package.json, node_modules, and .eslintrc.js.

If I cd into client and run ./node_modules/eslint/bin/eslint.js ., ESLint runs as expected.

If I try using git lint, then eslint runs into dependency issues. I have tried various things, but nothing has worked:

eslint:
  extensions:
  - .js
  command: cd
  arguments:
    - client
    - &&
    - ./node_modules/eslint/bin/eslint.js
    - .

won't run as it is invalid YAML, AFAIK. If I manually run said command, ESLint works as expected as I've mentioned above.

eslint:
  extensions:
  - .js
  command: ./client/node_modules/eslint/bin/eslint.js
  arguments:
    - client

gives error SKIPPED: ./client/node_modules/eslint/bin/eslint.js is not installed., but if I manually run said command I get Error: Cannot find module 'eslint-config-dev'.

Nomatter how I mix and match, I seem to have an issue. I'd rather not install ESLint and all the deps it needs globally (though I tried), as that leads to other issues. It's nice to have the correct deps and versions installed locally for the project.

sk- commented 5 years ago

I don't know what trickery is doing node and npm when you cd into a directory.

The easiest to fix your problem would be to write a wrapper script. Something like:

cd client
./node_modules/eslint/bin/eslint.js $1

and then in the config

eslint:
  extensions:
  - .js
  command: ./bin/wrapped-eslint.sh
MareoRaft commented 5 years ago

@sk- That sounds like a good idea. I have created a wrapper file as you suggested and made it executable. If I call the wrapper file from the terminal, it works as expected.

If I use:

eslint:
  extensions:
  - .js
  command: ./client/scripts/eslint.sh

I get SKIPPED: ./client/scripts/eslint.sh is not installed.. It seems to me that ANY executable will give me this output, with the exception of things in my $PATH.

So I tried instead:

eslint:
  extensions:
  - .js
  command: sh
  arguments:
    - ./client/scripts/eslint.sh
    - ./src

but in this case I get OK for the file, when in fact it does have an error which eslint normally picks up. Also, I made sure that the error is in my git diff, so that git lint would pick it up. I also tried plugging in ./src directly into the wrapper file, to make sure it wasn't an issue with the bash argument.

MareoRaft commented 5 years ago

Got it to work using

eslint:
  extensions:
  - .js
  command: sh
  arguments:
    - ./client/scripts/eslint.sh
    - ./src
  filter: "(?P<line>\\d+):(?P<column>\\d+)\\s+error\\s+(?P<message>.+)"

. I think my git-lint was a little out-of-date. After updating git-lint, I was able to change \\d+ in the regex back to {lines}, and it continued to work.

For completeness, the content of eslint.sh was:

#!/bin/sh
cd client
./node_modules/eslint/bin/eslint.js $1