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

Request: Lenient shell command parser #640

Closed
moos opened this issue Jun 25, 2019 · 7 comments
Closed

Request: Lenient shell command parser #640

moos opened this issue Jun 25, 2019 · 7 comments

Comments

@moos
Copy link

moos commented Jun 25, 2019

Description

It'd be great if normal bash command modifiers could be accepted (see below for example). I know same can be accomplished with an external script or an npm script -- but a one-liner would be super useful and less clutter.

Steps to reproduce

This example will fail the hook if there are any lingering .only()s left in staged test files. The ! is used to negate the exit code of grep.

  "lint-staged": {
    "src/**/*.test.js": [
      "!grep -q '\\.only\\W' "
    ]
  }

Debug Logs

...
Running tasks for src/**/*.test.js [started]
  lint-staged:make-cmd-tasks Creating listr tasks for commands [ "!grep -q '\\.only\\W' " ] +0ms
  lint-staged:find-bin Resolving binary for command `!grep -q '\.only\W' ` +349ms
Running tasks for src/**/*.test.js [failed]
→ !grep could not be found. Try `npm install !grep`.
Running linters... [failed]
!grep could not be found. Try `npm install !grep`.

This and not being able to position the file in the command, (e.g. grep -q '\.only\W' {}; test $? -eq 1 where {} is the (supposed!) file placeholder), is a real handicap in this otherwise very useful tool, IMHO.

Environment

  • OS: macOS High Sierra
  • Node.js: v11.13.0
  • lint-staged: v8.2.1
@okonet
Copy link
Collaborator

okonet commented Jun 29, 2019

We’re using execa to run commands. Please research if that’s possible and submit a PR that implements it.

@moos
Copy link
Author

moos commented Jun 30, 2019

This seems to be a limitation of child_process() which execa uses.

Also tried bash -c "! grep -q foo bar.js; echo $?" with the {shell: true} option. This gets around the command-not-found error, but alas, the negation fails to work in bash -c (though it works if run directly):

# assuming foo is in bar.js 
$ ! grep -q foo bar.js; echo $?  
1  # no-match (negated)

$ bash -c "! grep -q foo bar.js; echo $?"
0  # match (negation did not take)

For these reasons it's probably not worth pursuing further. The 2nd option (positioning the file argument) may be viable -- any thoughts on that?

@iiroj
Copy link
Member

iiroj commented Jun 30, 2019

@moos can you check out this test and if it works for you? ecd5aef#diff-7c51e29de830d755b8f565e11f965bd9

I think I was able to solve this by skipping parsing of the command string entirely when using --shell.

@moos
Copy link
Author

moos commented Jul 1, 2019

@iiroj Yes that worked -- I don't know how, but it did. Brilliant solution to both complex commands and filename placeholder!

// lint-staged.config.js:
module.exports = {
  "*.js": [
    () => "! grep foo bar.j"
  ]
};
// bar.js:
// I haz foo

Output:

  ❯ Running linters...
    ❯ Running tasks for *.js
      ✖ () => "! grep foo bar.js"

✖ () => "! grep foo bar.js" found some errors. Please fix them and try committing again.
// I haz foo

Removing the ! is successful. Compound commands also works:

  "*.js": [
    () => "grep foo bar.js; test $? -eq 1"
  ]

@okonet
Copy link
Collaborator

okonet commented Jul 1, 2019

Closed in #639

@lkraav
Copy link

lkraav commented Aug 26, 2020

Very nice, --shell --verbose works 🚀 Here's the solution for integration phpcbf, which returns a relatively non-standard exit code 1, if it successfully fixes any code squizlabs/PHP_CodeSniffer#1359

"husky": {
  "hooks": {
    "pre-commit": "lint-staged --shell --verbose"
  }
},
"lint-staged": {
  "*.php": [
    "! ./vendor/bin/phpcbf",
    "./vendor/bin/phpcs -s"
  ]
}

The ! is used to negate the exit code of grep.

PS nice idea @moos ☝️

@lkraav
Copy link

lkraav commented Aug 27, 2020

The ! is used to negate the exit code of grep.

PS nice idea @moos point_up

Not so simple with ! after all - obviously it also negates exit code 0. Had to convert to lint-staged.config.js and build my own command-line:

const path = require('path');

module.exports = {
    '*.php': (filenames) => {
        // @see https://github.com/okonet/lint-staged#example-use-relative-paths-for-commands
        const cwd = process.cwd();
        const relativeFilenames = filenames.map((file) => path.relative(cwd, file)).join(' ');

        return [
            `./vendor/bin/phpcbf ${relativeFilenames}; if [ $? -eq 1 ]; then exit 0; fi`,
            `./vendor/bin/phpcs -s ${relativeFilenames}`,
            `./vendor/bin/psalm ${relativeFilenames}`,
        ]
    }
}

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

No branches or pull requests

4 participants