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

Is it still alive? Some advantages are needed. #7

Open tolezk opened 2 years ago

tolezk commented 2 years ago

Hi, @idanpa and thank you for the extension!

So, long story short, I need this to have some additional features:

I'm not a person who familiar with js, ts or vscode-extensions creation. But I can provide a pull-request where I'll try to do my best to implement these features. It may violate typical codestyle or common sense for ts, though.

Now, some details.

The idea behind the first feature is to implement an alternative parseCheckpatchLog() function (or addition to it) which may handle a different output from checkpatch. For comparison:

WARNING:LINUX_VERSION_CODE: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
#124: FILE: mac.c:124:
+#if KERNEL_VERSION(5, 8, 1) <= LINUX_VERSION_CODE

- this style of logs is supported for now.

mac.c:124: WARNING:LINUX_VERSION_CODE: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
+#if KERNEL_VERSION(5, 8, 1) <= LINUX_VERSION_CODE

- This style I want to be supported (here --showfile option was passed to checkpatch).

About second feature. I think it would be good to define some option that a user can define inside settings.json of a particular workspace, and then extension should use this workspace as cwd, but not the root. I don't know if this possible or some other approach should be choosen. Of course this related to the Checkpatch Selected File feature.

I need some feedback for it from you, Cheers.

idanpa commented 2 years ago

Hi @tolezk, thanks for opening this issue!

  1. --showfile - seems reasonable to support this flag, feel free to open a PR (need to adjust the regex if showfile arg was given).
  2. It is possible to adjust the cwd of the checkpatch process, but seems overcomplicated to me. checkpatch would run from the root dir of your project, so you can have the .checkpatch.conf file waiting for it in there, alternatively, you can adjust checkpatch.checkpatchArgs on settings.json if you want to adjust the flags for specific workspace/globally.
tolezk commented 2 years ago

I've made a PR (https://github.com/idanpa/vscode-checkpatch/pull/8) to cover 1. '--showfile'.

Now some thoughts about 2: run checkpatch for a particular file.

I think it should be similar to checkpatchCommit() in terms of CWD inheritance etc. Running a linter from the root workspace is a quite basic use-case because a project workspace layout may be somehow tricky. For example:

vscode_workspace:
|
|--root_workspace (some complex project with many directories and subprojects inside)
|  |--dir1
|  |
|  |--dir2
|  |
|  |--dir3
|  |
|  |--subproject_A
|  |
|  |--subproject_B
|  |
|  |--scripts/checkpatch.pl (this one may be a non-standard somehow modified to cover custom requirements)
|  |--.checkpatch.conf
|  
|--subproject_A_of_root_workspace  
|  |--dir1
|  |
|  |--dir2
|  |
|  |--.checkpatch.conf
|
|--subproject_B_of_root_workspace  
|  |--dir1
|  |
|  |--dir2
|
|--linux
   |--arch
   |
   |--... (all the kernel stuff)
   |
   |--scripts/checkpatch.pl
   |--.checkpatch.conf

Here we have:

To keep things playable in even a such case my proposal is to let a user define folder-level settings inside each folder, where required. And it still would keep backward compatibility with the current implementation. And would give the flexibility to achieve these goals.

Some details. Several options (checkpatchPath, checkpatchArgs, exclude) I want to switch to 'resource' scope. Also, I want to introduce a new option useWorkspaceAsCwd with the same scope which would let to consider the folder it applied to as CWD.

tolezk commented 2 years ago

I want you to confirm that proposal is ok to you and I will prepare the next PR to implement. Or we may have some discussion about.

tolezk commented 2 years ago

Anyway, I've implemented all the stuff. Now preparing a new PR.