Wingman/tactics "hole_severity" option is broken

[mirko-plowtech]

VS code doesn't allow to setup number options due to a "Invalid type. Expected string"

• from https://github.com/haskell/vscode-haskell/blob/master/package.json#L260

        "haskell.plugin.tactics.config.hole_severity": {
          "enumDescriptions": [
            "error",
            "warning",
            "info",
            "hint",
            "none"
          ],
          "scope": "resource",
          "description": "The severity to use when showing hole diagnostics.",
          "enum": [
            1,
            2,
            3,
            4,
            null
          ],
          "default": null,
          "type": "string"
        },

^ 1,2, 3, 4 are integers, but "validation" requires string.

• Op 1. It should use integer (btw, instead of null it could use 0)
• Op 2. It should use "1","2", "3", "4"

[jneira]

Many thanks for noting it, good catch!

I have to figure what could be the values matching the language server config type (Maybe DiagnosticSeverity):

https://github.com/haskell/haskell-language-server/blob/ce1f353be354711d905f287d1b22f24c06606409/plugins/hls-tactics-plugin/src/Wingman/LanguageServer.hs#L150

Would you like and have time to open a pr to fix it?

[jneira]

Ping @berberman as you have the expertise about the server and client config.
I think we could replace the enum keys with the lower case version of the haskell enum (so actual enumDescriptions would work afaiu), but not sure how to make Maybe a, Nothing without using null

[mirko-plowtech]

Many thanks for noting it, good catch!

I have to figure what could be the values matching the language server config type (Maybe DiagnosticSeverity):

https://github.com/haskell/haskell-language-server/blob/ce1f353be354711d905f287d1b22f24c06606409/plugins/hls-tactics-plugin/src/Wingman/LanguageServer.hs#L150

Would you like and have time to open a pr to fix it?

Sure, I can try. I will probably work on it this weekend =)

[mirko-plowtech]

Many thanks for noting it, good catch!
I have to figure what could be the values matching the language server config type (Maybe DiagnosticSeverity):
https://github.com/haskell/haskell-language-server/blob/ce1f353be354711d905f287d1b22f24c06606409/plugins/hls-tactics-plugin/src/Wingman/LanguageServer.hs#L150
Would you like and have time to open a pr to fix it?

Sure, I can try. I will probably work on it this weekend =)

@jneira Sorry, I tried to fix it, but I guess I failed in the attempt.

I realized that with any config I tried to use, Wingman suggestions are always the same...Tbh, at this point, I'm not sure if I understood correctly how this work, I mean, what behavior should this option modify?)

Btw I tried using string with the following options:

[ "DsError", "DsWarning", "DsInfo", "DsHint", null]

or

   [ "error", "warning", "info", "hint", null]

and also number with:

[1, 2, 3, 4, 0]

^ Despite the error doesn't appear anymore, I didn't find any difference, so probably those options won't work..

[jneira]

I realized that with any config I tried to use, Wingman suggestions are always the same...Tbh, at this point, I'm not sure if I understood correctly how this work, I mean, what behavior should this option modify?)

many thanks for checking it, i am afraid my knowledge about wingman internals is very limited, maybe @berberman or @isovector could help us to understand the config values or how to effectively test they are bein taken in account

[isovector]

hole_severity is the severity wingman uses to tag holes. The difference between the values is whether or not they show up as warnings, errors or hints in the Diagnostics page in vs-code. But, this feature is working for me in vs-code:

[isovector]

Oh, just kidding. I see it now. This is definitely a bug in the property parser, not in Wingman.

[isovector]

Here's the bug: https://github.com/haskell/haskell-language-server/blob/4b7d13977361c515dad8b2e83f50503ea779150d/hls-plugin-api/src/Ide/Plugin/Properties.hs#L424

This should instead be a function of the enum type? Or maybe it should always be integer?

[isovector]

This is a regression caused by me in haskell/haskell-language-server#1608. I'm not sure what the best fix is though. The test case there uses a string as the JSON serialization, so it's not good enough to always be an integer.

[jneira]

Maybe we should change the values here, making them fit the parser requirements (i think for enums it should accept the enum constructor names all in lower case)
But @mirko-plowtech tried several alternatives included the mentioned one and does not see a evident difference, maybe with you explanation is easier to check if it works

[isovector]

Not sure about the parser --- this instance is defined upstream

https://github.com/haskell/lsp/blob/bf95cd94f3301fe391093912e6156de7cb5c1289/lsp-types/src/Language/LSP/Types/Diagnostic.hs#L20-L40

and the spec seems to agree that this should be an integer:

https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#diagnosticSeverity

[mirko-plowtech]

Not sure about the parser --- this instance is defined upstream

https://github.com/haskell/lsp/blob/bf95cd94f3301fe391093912e6156de7cb5c1289/lsp-types/src/Language/LSP/Types/Diagnostic.hs#L20-L40

and the spec seems to agree that this should be an integer:

https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#diagnosticSeverity

Right, it should use integer instead of string. I tested it locally and it works, here is the PR: #511