joaotavora / eglot

A client for Language Server Protocol servers
GNU General Public License v3.0
2.22k stars 203 forks source link

wrap (buffer-substring-no-properties (point-min) (point-max)) with "eglot--buffer-content" #1067

Closed zhenhua-wang closed 1 year ago

zhenhua-wang commented 1 year ago

Currently, Elot is restrictive in managing buffer content. This could be an issue when a user wishes to choose which portions of the buffer to send to the server (e.g. in Markdown/OrgMode's code block, I only want to sent the code not the markdown text). By wrapping (buffer-substring-no-properties (point-min) (point-max)) with eglot--buffer-content, user can add any advice to eglot--buffer-content function.

zhenhua-wang commented 1 year ago

Maybe a better logic is to replace all buffer-substring-no-properties functions with something like eglot--region-content with (point-min), (point-max) supplied as optional arguments? Any idea on this?

joaotavora commented 1 year ago

I have many doubts, but the main one is: what if the server is not using "full sync" on every file change?

You're effectively proposing to "lie" to the server: it thinks the file has some content, when in reality it has some other content. How can this work?

Can you show an actual application of this?

zhenhua-wang commented 1 year ago

You're effectively proposing to "lie" to the server: it thinks the file has some content, when in reality it has some other content. How can this work?

Can you show an actual application of this?

In typical software development, this might be rare. But in literate programming (e.g. statistics, data science), this is quite common. For example, a Rmarkdown file contains markdown texts and R codes. Sending both markdown texts and codes to the server would cause many issues (e.g. missing workspace variables, wrong code linting).

This is an example of how it works in lsp-mode. It basically replaces the content of lsp--buffer-content with only codes in R code blocks.

I have many doubts, but the main one is: what if the server is not using "full sync" on every file change?

That's why I was proposing eglot--region-content, which is basically an alias for buffer-substring-no-properties. Perhaps there might be another good way to define this function. Below is just an rough idea.

(defalias 'eglot--region-content 'substring-no-properties)

As an user, I definitely don't want to contaminating the built-in buffer-substring-no-properties function, as it might break other stuff in emacs.

joaotavora commented 1 year ago

I understand the idea, but frankly, i can't see how it can work. Have you actually used this PR successfully for literate programming with LSP?

zhenhua-wang commented 1 year ago

Yes, I test it with the following codes to control the buffer contents sent to the server. It might need a bit of work to figure out the "non-full sync" case, but that's not an issue for eglot but an issue for polymode.

  ;; send polymode content to eglot
  (defun zw-polymode-lsp-buffer-content (orig-fun &rest arguments)
    (if (and polymode-mode pm/polymode)
        (pm--lsp-text)
      (funcall orig-fun arguments)))
  (advice-add #'eglot--region-content
              :around #'zw-polymode-lsp-buffer-content)
joaotavora commented 1 year ago

I don't understand what happens when the server sends a diagnostic. Where is it annotated? What about go-to definition? All the live/column coordinates on Emacs side make no sense in the lsp side.

I'll be honest: If you want this to have half a chance, you need to show a full recipe and demonstration so that I can see it in action in my own machine. If it works well, I'll be surprised, and quite happy :)

zhenhua-wang commented 1 year ago

I see your confusion now. Well, what polymode did was just replacing all the markdown text with empty lines, so it won't cause any problems when doing completion, go-to definition etc. But when it comes to diagnostic, it would generate "Trailing blank lines" messages. Sorry I didn't catch this, because I don't use flymake in polymode. I will close it for now and keep this PR as a hack in my config...