CoffeeChaton / vscode-autohotkey-NekoHelp-Old

my style
Other
9 stars 0 forks source link

Overzealous linting #4

Closed Lexikos closed 1 year ago

Lexikos commented 2 years ago

I would suggest providing more options to disable or tweak specific warnings, as some of them are excessive. (I've found the general lack of control over warnings in VS Code to be a problem with other extensions as well.)

It is completely legitimate and not uncommon to write switch with no default: case. There should be no warning for this.

Warnings for "deprecated" commands show strike-through (a line through the middle of the word) as well as the yellow squiggle, which is too much. Usually it's quite easy to see that a deprecated command is being used, and when I'm reading old code, I don't need the editor screaming at me about it.

FileRead and FileAppend are not deprecated. These commands are more efficient than the alternatives, and are still present in v2 (but are functions). There are plenty of cases where legacy syntax must be used in v1, so I think warning about the use of these commands isn't productive. If someone prefers to use FileOpen instead to avoid the command syntax, they probably don't need a reminder.

I think warnings about unused parameters are more distracting than useful, especially for callback functions. It is not uncommon to define the parameters for an OnMessage function and not use all of them, for instance.

Lexikos commented 2 years ago

Regarding the default: case or lack thereof, even if present, the extension gives two warnings if there is no space before the colon.

The first warning ("Default : not find", also present if there is no default case) covers the entire switch, which obscures the fact that there is a second warning.

The second warning appears only in the problem pane or if you hover over the default case itself:

did you mean switch case default : ? this way look like a label:.

It now becomes clearer what issue the extension has taken to the code: it wants to see a space before the colon. I have no objection to including a space if that is your personal style, but requiring one by default in a linting tool is nonsense. The most common style, and the one shown in the official documentation, is to not use a space.

It is supposed to look like a label, which is why it requires a colon.

If you insist on retaining these warnings, please make them optional, and please change them to underline only a small portion of the code, such as the keyword switch or default:.

CoffeeChaton commented 1 year ago

I would suggest providing more options to disable or tweak specific warnings, as some of them are excessive.

I will provide 4 options to provide user choice

  1. none diagnose , some users no need diagnose.
  2. (default) if (workspaceFolders is open) {diag file} esle {not diag} , temporary editing or just reviewing, no need to diagnose
  3. Always diagnose, regardless of whether there is no open workspaceFolders 3. Always diagnose and add some diagnostic options that I need

(I've found the general lack of control over warnings in VS Code to be a problem with other extensions as well.)

Deeply agree, especially the filter list below has not been very good to use.

It is completely legitimate and not uncommon to write switch with no default: case.

I know it's legal, but think it's more of a refactoring error, and this diagnostic option will then be moved to the personal use diagnostic collection(Options 3).

delete this ruler.

deprecated

deprecated diagnostic , will move to Options 3 at this weekend. Provides independent options.

{
    "AhkNekoHelp.Diag.code800Deprecated": false
}

FileAppend, FileGetAttrib, FileRead, StringLower,StringUpper

Configuration error, will fix this at this weekend.

Default : not find and did you mean switch casedefault :? this way look like a \label:`.`

is parsing error , will move to Options 3 .

CoffeeChaton commented 1 year ago

add some option to user.

// settings.json // https://stackoverflow.com/questions/65908987/how-can-i-open-visual-studio-codes-settings-json-file
{
    "AhkNekoHelp.Diag.AMasterSwitch": "auto",
    "AhkNekoHelp.Diag.code107LegacyAssignment": true,
    "AhkNekoHelp.Diag.code300FuncSize": 100,
    "AhkNekoHelp.Diag.code800Deprecated": true
}