Saghen / blink.cmp

Performant, batteries-included completion plugin for Neovim
MIT License
632 stars 27 forks source link

feat: add detail to documentation window #33

Closed telebart closed 12 hours ago

telebart commented 2 days ago

Stylize the documentation window

Before: Screenshot 2024-10-08 at 17 06 14 Screenshot 2024-10-08 at 17 06 02 After: Screenshot 2024-10-08 at 17 06 36 Screenshot 2024-10-08 at 17 06 27

julienvincent commented 2 days ago

I have one gripe with this change:

I was using nvim-cmp before and it would constantly break trying to render documentation coming from clojure-lsp. The failure came from calling the stylize_markdown function you are using in this change. See example below:

image

If this lands I would suggest either:

  1. Catching exceptions from calling this (using pcall or something) and then falling back to the existing behaviour.
  2. Sticking it behind some config that can be turned off.

Otherwise, this looks awesome!

🙏🏻

telebart commented 2 days ago

Hmm, I guess having stylize_markdown behind pcall would be a sensible thing to do since this isn't probably the only edge case considering markdown and lsp standards. I'm interested though what is causing it since the error seems quite straightforward nil exception and could probably be fixed in upstream to utils. Sadly I couldn't find a setup to reproduce that error with my limited knowledge of clojure.

sudo-tee commented 2 days ago

I was also trying to get this work and found 2 things that could help

you could check item.documentation.kind == "markdown" instead of a table.

The second part is for some reason if a code block does not end in it's own newline it will break vim.lsp.stylize_markdown
image

but if you replace the code with something like this doc = doc:gsub('```\n', '\n```\n') it works properly

image

julienvincent commented 2 days ago

Sadly I couldn't find a setup to reproduce that error with my limited knowledge of clojure.

To reproduce:

  1. Make sure you have clojure-lsp configured (the defaults from nvim-lspconfig work perfectly fine)
  2. Use this PR's branch
  3. Make and edit a clojure file called example.clj
  4. Add the following:
(ns example
  (:require
   [clojure.test :as t]))

(t/*testi|) ;; trigger autocomplete here
  1. Success :)

Triggering autocomplete there will give you an immediate failure. I know I have reproduced this in other languages in the past but I wasn't able to get a reliable reproduce sequence now. Sorry for the clojure :)

telebart commented 2 days ago

I was also trying to get this work and found 2 things that could help

you could check item.documentation.kind == "markdown" instead of a table.

The second part is for some reason if a code block does not end in it's own newline it will break vim.lsp.stylize_markdown

Thanks for looking into this. This seems like a malformed value. I would not like to fix it here since it feels weird to do this kind of duct tape solutions when there is something wrong with upstream

  documentation = {
    kind = "markdown",
    value = "```lua\nlocal status, err_or_value = pcall(function)```\n---\nProtect call a function as a variable"
  }
5. Success :)

Couldn't succeed with failing but now that I think about it I might have seen the error message flash before with other languages.

Sorry for the clojure :)

It's more of a problem of loose standards and string parsing that is at fault here. Nothing to be sorry about using a language

I would propose that this PR would just try the stylizing (If it's wanted/needed feature) from the lsp utils library and falls back to simpler one. Other things would be out of scope and subject for another PR.

Added fallback and removed hanging printing statements. Simple formatting:

Screenshot 2024-10-09 at 7 50 21
telebart commented 1 day ago

After testing this for a while I realized that I really only needed the item.detail for the documentation window. Sorry for changing the scope.

Saghen commented 12 hours ago

Thanks!