idanpa / vscode-checkpatch

Visual Studio Code extension for using linux kernel checkpatch tool to lint code.
https://marketplace.visualstudio.com/items?itemName=idanp.checkpatch
MIT License
11 stars 4 forks source link

Extension improvements #2

Closed nunojsa closed 4 years ago

nunojsa commented 4 years ago

This PR adds two fixes:

  1. Adding some arguments would make the loadconfig function to fail;
  2. Files with only one problem would never be updated after fixing the problem.

Additional, it adds support to the --strict argument so that new messages can be displayed.

idanpa commented 4 years ago

Thank you @nunojsa! 💐 I'll try to test this and publish new version over the weekend. Could you please elaborate on 1? I wanted loadconfig to also check the arguments. What argument was causing it to fail?

nunojsa commented 4 years ago

Thank you...

No problem, that's open source :+1:

I think --strict is causing it. I always use this option to prepare my linux patches that's why I came across this issue....

idanpa commented 4 years ago

Everything looks fine but I want loadconfig to test the arguments, and to fix the root cause of the problem. Couldn't recreate the loadconfig error with --strict. What flags are you using? What is the output of echo "" | checkpatch.pl --no-tree --strict - with your flags?

nunojsa commented 4 years ago

That is odd. I will have to try it again. Not sure if I will have time this weekend but maybe tomorrow I can have a look on this and give you more feedback

nunojsa commented 4 years ago

So, I had the time to try this out. It happens as soon as I had --strict to the settings...

image

These are my settings:

{
    "checkpatch.checkpatchPath": "/home/nsa/work/linux/scripts/checkpatch.pl",
    "checkpatch.checkpatchArgs": [
        "--strict"
    ]
}

This is what I get when running echo "" | checkpatch.pl --no-tree --strict -

ERROR: Does not appear to be a unified-diff format patch

total: 1 errors, 0 warnings, 0 checks, 0 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

As I said in the commit message it seems the regex used is strictly checking against /total: \d* errors, \d* warnings, \d* lines checked/g. When using --strict we have a different line. Im using version 0.32

idanpa commented 4 years ago

You are absolutely right, merged the fixes, thanks again. I will make loadConfig to check user arguments but respect --strict.