problem_on_unmodified_line should render errors on unmodified lines as a warning color

[cihati]

Hi -- thanks for the great tool! As a disclaimer, I already took a look at #240 and #638

I'm using overcommit 0.53.0. This is my .overcommit.yml:

verify_signatures: false

PreCommit:
  ALL:
    problem_on_unmodified_line: warn
  Pycodestyle:
    enabled: true
#    on_fail: warn
  Pydocstyle:
    enabled: true
#    on_fail: warn
  Pyflakes:
    enabled: true
#    on_fail: warn
  Pylint:
    flags: ['--config', 'layer-platform/.pylintrc']
    enabled: true
#    on_fail: warn
  Hadolint:
    enabled: true
#    on_fail: warn
  BranchNameValidation:
    enabled: true
    command: './custom-git-hooks/branch-name-validation'
PrePush:
  CircleCIValidation:
    enabled: true
    command: './custom-git-hooks/circleci-validation'
    required_executable: 'circleci'
    install_command: 'brew install circleci'
    include: '.circleci/config.yml'
  TerraformFormatting:
    enabled: true
    command: './custom-git-hooks/terraform-formatting'
    required_executable: 'terraform'
    install_command: 'brew install terraform'
    include: '**/*.tf'

I just added the problem_on_unmodified_line: warn line. My expectation is that errors for the lines I did not modify would be shown as warnings. However, that's not the case:

$ git commit -am "test"
Running pre-commit hooks
Run BranchNameValidation.......................[BranchNameValidation] OK
Analyze with pyflakes......................................[Pyflakes] OK
Analyze with pycodestyle................................[Pycodestyle] FAILED
Errors on modified lines:
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:453:121: E501 line too long (134 > 120 characters)
Errors on lines you didn't modify:
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:509:121: E501 line too long (131 > 120 characters)
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:522:121: E501 line too long (141 > 120 characters)
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:587:121: E501 line too long (124 > 120 characters)
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:599:121: E501 line too long (141 > 120 characters)
/Users/cihat/layer/backend/layer-platform/backend/components/component.py:602:121: E501 line too long (147 > 120 characters)

Analyze docstrings with pydocstyle.......................[Pydocstyle] OK
Analyze with Pylint..........................................[Pylint] OK

✗ One or more pre-commit hooks failed

I have already committed my .overcommit.yml changes:

$ git status
On branch cihat/fix-overcommit
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   layer-platform/backend/components/component.py

no changes added to commit (use "git add" and/or "git commit -a")


$ git show
d03c7b0581fbe5064978ca220a91c314ad06e0fc (HEAD -> cihat/fix-overcommit) Overcommit update

.overcommit.yml


18
 verify_signatures: false

 PreCommit:
   ALL:
     problem_on_unmodified_line: warn
   Pycodestyle:
     enabled: true
     on_fail: warn
 #    on_fail: warn
   Pydocstyle:
     enabled: true
     on_fail: warn
 #    on_fail: warn
   Pyflakes:
     enabled: true
     on_fail: warn
 #    on_fail: warn
   Pylint:
     flags: ['--config', 'layer-platform/.pylintrc']
     enabled: true
     on_fail: warn
 #    on_fail: warn
   Hadolint:
     enabled: true
     on_fail: warn
 #    on_fail: warn
   BranchNameValidation:
     enabled: true
     command: './custom-git-hooks/branch-name-validation'

problem_on_unmodified_line: ignore works as intended.

Any ideas?

[sds]

What happens if you fix the error on the modified line? Do you still get a FAILED status? Or does it then change to WARN?

[cihati]

Before I fix the error on the modified line:

After I fix the error on the modified line:

So, I think the issue is that the errors on unmodified lines in first image should be yellow and shown as maybe "Warnings on lines you didn't modify:"

[sds]

Apologies for the delay in response. Missed this in my notifications.

Overcommit has two classes of hook results: FAILED and WARNING; and two classes of issues/messages that can be reported within a hook: Error and Warning. The problem_on_unmodified_line setting allows you to "downgrade" what would otherwise be a FAILED hook to a WARNING if any Errors were reported on lines you did not modify. This is beneficial since if someone else commits something which would cause your local checks to fail, your local hook run will still pass with WARNING as long as you didn't modify those lines.

I would like to suggest that the wording remain as-is for this reason. These are still "errors", but the hook treats them as a final status reported of WARNING since they are on unmodified lines.

As for making the errors on on modified lines appear as yellow: will leave this open as a feature request. Thanks for the feedback!