Skip to content

Conversation

@seckenrode
Copy link
Contributor

The MESSAGE_REGEX for stylelint does not appropriately extract
line numbers >= 10. This can cause the hook to incorrectly report
errors on modified lines as being for lines that are not modified.

This commit changes the regex to correctly handle multiple digits in
line numbers.

Reproduction Steps

  1. Create an empty git repository
  2. Install overcommit with the following configuration in .overcommit.yml in the repository
    PreCommit:
      Stylelint:
        enabled: true
        command: "stylelint"
        problem_on_unmodified_line: "warn"
    
  3. Install stylelint and create a .stylelintrc.json file in the repository:
    {
      "rules": {
        "declaration-block-trailing-semicolon": "always"
      }
    }
    
  4. Create a file named failing_file.scss in the repository with the following content:
    .selector-1 { width: 20%; }
    .selector-2 { width: 20%; }
    .selector-3 { width: 20%; }
    .selector-4 { width: 20%; }
    .selector-5 { width: 20%; }
    .selector-6 { width: 20%; }
    .selector-7 { width: 20%; }
    .selector-8 { width: 20%; }
    .selector-9 { width: 20%; }
    .selector-10 { width: 20%; }
    .selector-11 { width: 20%; }
    
  5. Commit these files. Overcommit should pass.
  6. Modify line 11 of failing_file.scss to be the following:
    .selector-11 { width: 20% }
    
  7. Run git commit -a. This should fail with an error on the modified line. Instead, it passes with a warning on lines you did not modify:
    $ git commit -a
    Running pre-commit hooks
    Check styles with Stylelint...............................[Stylelint] WARNING
    Errors on lines you didn't modify:
    /Users/seckenrode/development/stylelint-test/failing_file.scss: line 11, col 25, error - Expected a trailing semicolon (declaration-block-trailing-semicolon)
    

After this change, the check fails with an error as expected.

seckenrode and others added 2 commits June 4, 2019 13:31
The MESSAGE_REGEX for stylelint does not appropriately extract
line numbers >= 10.  This can cause the hook to incorrectly report
errors on modified lines as being for lines that are not modified.

This commit changes the regex to correctly handle multiple digits in
line numbers.
@seckenrode
Copy link
Contributor Author

Thanks for fixing the overcommit warning -- hoisted by my own auto-pilot for our internal overcommit configuration :)

@sds
Copy link
Owner

sds commented Jun 4, 2019

AppVeyor failures seem unrelated (looks like an issue in the CI pipeline). Since this doesn't make Windows-specific changes, going to merge.

@sds sds merged commit bc738ab into sds:master Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants