phpro / grumphp

A PHP code-quality tool
MIT License
4.14k stars 429 forks source link

stylelint task only adds files in GitPreCommitContext #926

Closed leonexcc closed 2 years ago

leonexcc commented 3 years ago

Is there a reason this was implemented like this: https://github.com/phpro/grumphp/blob/4248487a8f34188aec5d48b767e02ccea432a028/src/Task/Stylelint.php#L122-L127

It will produce an error if i run grumphp run outside of a git commit (since stylelint need files to run on).

I comparison the eslint task doesn't do this and will always add the files: https://github.com/phpro/grumphp/blob/4248487a8f34188aec5d48b767e02ccea432a028/src/Task/ESLint.php#L80-L82

leonexcc commented 3 years ago

I just saw this was discussed in https://github.com/phpro/grumphp/pull/904#discussion_r646086110. But i don't understand how stylelint would work without files:

  Usage: stylelint [input] [options]

  Input: Files(s), glob(s), or nothing to use stdin.

    If an input argument is wrapped in quotation marks, it will be passed to
    globby for cross-platform glob support. node_modules are always ignored.
    You can also pass no input and use stdin, instead.

I also didn't find any possibility to define files or folders in the .stylelintrc file

veewee commented 3 years ago

I'm not using the tool myself. Maybe @yguedidi can elaborate?

leonexcc commented 3 years ago

We made a copy of the task with the same behaviour as in the ESLint task and it works for us fine. If you want, i can make a pull request with the change.

There is probably a problem with projectes with many css or scss files. But you have probably the same problem with eslint.

veewee commented 3 years ago

Feel free to suggest a PR. We can continue the discussion from that point in.

veewee commented 2 years ago

Will be fixed in #932

yguedidi commented 2 years ago

Hello @leonexcc @veewee! First, sorry to answer so late! I confirm it was a mistake from me :sweat_smile:

That said, I just contributed #976 so that TwigCS don't run full lint on precommit but only on changed files. Maybe the same pattern could be used here? have a path option to be used by default on run context, and use changed files on precommit context. what do you think?

leonexcc commented 2 years ago

Hello @yguedidi, i think a path option would be great.