nushell / vscode-nushell-lang

A Nushell grammar for Visual Studio Code with IDE support
https://www.nushell.sh/
MIT License
121 stars 27 forks source link

:bug: fix markdown formatting for hover lsp #175

Closed AucaCoyan closed 9 months ago

AucaCoyan commented 9 months ago

Hi, I wandered around for a bit and found out that when the server returns a hover, you have to inform the client which type of plaintext | markdown you are returning.

So, instead of

      const contents = {
        value: obj.hover, // actual hover string
        language: "nushell",
      };

it should be

      const contents = {
        value: obj.hover, // actual hover string
        kind: "markdown"
      };

(note that both key and value changed)

and you get the result 😄 image

By removing the language: nushell key, it defeats me how vscode knows that the code block (```) parts of the hover are in nushell syntax, because that is not written in the raw hover string 🧠 .

I hope this doesn't break any other editor that doesn't have markdown syntax!

fdncred commented 9 months ago

Thanks @AucaCoyan! I had 2 thoughts myself on this.

  1. was the ``` were in the wrong place
  2. markdown just like this PR. (i was looking at this https://stackoverflow.com/questions/72002634/markdown-not-working-in-hover-content-in-lsp)

I guess it knows the language by the file type you have loaded maybe? Nice work!

AucaCoyan commented 9 months ago

Yeah, I followed up those same steps, when 1st didn't solved the problem, I checked that markdown type of response

fdncred commented 8 months ago

I'm guessing there's still some problem with this because this is what I see on my mac this morning. Screenshot 2024-03-07 at 6 34 05 AM

I'm wondering if there's some line ending thing that is causing this to not work right. I'll try to check on linux and windows later.

fdncred commented 8 months ago

hmmm, still looks broken in Ubuntu as well. Screenshot 2024-03-07 at 6 55 58 AM

fdncred commented 8 months ago

ugh, windows too. I wonder what the deal is? it was working. image

bfren commented 8 months ago

I know this is merged, but I'm getting this on my machine (Windows) - and have been for a while - is there a fix?

fdncred commented 8 months ago

@AucaCoyan was going to look into it last we talked. I assume he just hasn't got around to it yet.

AucaCoyan commented 8 months ago

Hello! I tried some simple stuff, like removing the 3 backticks (`) from the ide hover and it didn't solved the problem. From there, I didn't had the chance to investigate further

bfren commented 8 months ago

Do we know what actually broke it / which release it broke for?

fdncred commented 8 months ago

Do we know what actually broke it / which release it broke for?

Someone may have to do a git bisect to see what broke it. Either that or just play around with it until they show up right. My guess is that either the backticks are messed up or the error type is somehow set to string, even though we're calling it markdown.

AucaCoyan commented 8 months ago

Yep, I can do that the next time I sit with that problem. I will make time this weekend 😄 If anyone else is curious, bear in mind that vscode-nushell-lang serves from nushell, so maybe the markdown can be broken not by changes of this repo, but for changes in the nu --ide-hover from nushell/nushell. We have a two (or more) headed hydra here