lvimuser / lsp-inlayhints.nvim

Apache License 2.0
423 stars 23 forks source link

adjust arguments order to match conventions #13

Closed alex-popov-tech closed 2 years ago

alex-popov-tech commented 2 years ago

Hey! I had some weird errors, and after some debugging i noticed that you actually declared your on_attach func with (bufnr, client) args, wherefore in lsp docs its on_attach(client, bufnr):

{on_attach}          Callback (client, bufnr) invoked when client
                              attaches to a buffer.

you can check it yourself :h lsp

lvimuser commented 2 years ago

Thanks!

Could you also change the annotation order? While we're at it might also change set_store.

Since it's a breaking change, I think it's best if we add some deprecation notice.

Something like this on M.on_attach

  -- TODO Remove
  if type(bufnr) == "table" then
    vim.notify_once(
      "[LSP Inlayhints] on_attach should be called with (client, bufnr)",
      vim.log.levels.WARN
    )

    client, bufnr = bufnr, client
  end
alex-popov-tech commented 2 years ago

Sure! Also if this pr is not about on_attach, maybe you won't mind I will try to add an ability to provide a function instead ( or as an alternative) to suffix? Like func (hint) string which should return a formatted hint?

lvimuser commented 2 years ago

Sure! Also if this pr is not about on_attach, maybe you won't mind I will try to add an ability to provide a function instead ( or as an alternative) to suffix? Like func (hint) string which should return a formatted hint?

Do note that there's some string manipulation going on, and the options are mostly a result of extmarks' limitation (can't set inline).

Once we have anticonceal, we can remove at least half of the code (loops/filtering), and these will options will be gone (in favor of a formatted func like you said).

It may not be worth the effort for now, but it's something to keep in mind.

I'll accept a PR, just letting you know.

Possibly return an extmark's virt_text (which is a tuple { text, highlight })? So the signature would look like func (hint) -> table virt_text.

alex-popov-tech commented 2 years ago

that makes sense! i will make this one related to only cliend, bufnr for now :)

lvimuser commented 2 years ago

Thanks!