search

joaotavora / eglot

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

Support for ccls's skipped region notifications. #1343

Closed coffeemug closed 9 months ago

coffeemug commented 9 months ago

CCLS publishes skipped region notifications for inactive code. An example of this is C preprocessor-- if you're in the middle of an $ifdef SOME_PLATFORM ... #endif block and SOME_PLATFORM isn't defined, CCLS informs the client that the section is inactive. This pull request adds support for these notifications, which allows for a nicer developer experience.

Eglot + ccls without this pull request:

image

Eglot + ccls with this pull request:

image

In big c/c++ files, this hinting can make enormous difference in dev experience quality. As an aside, clangd also supports this via an extension to semantic highlighting, but that would be a different pull request.

@joaotavora -- is it of interest to get this merged? (In any case thanks for your work on eglot and making Emacs better!)

joaotavora commented 9 months ago

Thanks, but Eglot is, by design, only for LSP nor for extensions to the protocol. Your code is very similar to precisely the example presented for the same feature for clangd in the manual: https://joaotavora.github.io/eglot/#Extending-Eglot-1

Your code is both simpler and more complicated. Simpler in that it uses remove-overlays, which is clever (no separate variable needed) and more complicated in that it uses an explicit iteration variable when it could have used a sequence-traversing primitive such as mapc or cl-loop (the latter gives destructuring and sequence iteration in one go).

So in summary, I suggest you:

  1. Submit this source to eglot-x.el which is maintained by @nemethf . THe url is https://github.com/nemethf/eglot-x
  2. Also submit this example to Eglot's manual, integrating it with the existing one. The manual, like the Eglot source code itself, now lives in Emacs's upstream
coffeemug commented 9 months ago

Thanks, will do. FYI-- I didn't know about eglot-x, it doesn't seem to be very discoverable. May be worth linking to it in README.md.

joaotavora commented 9 months ago

README.md is a still GitHub thing, so a GH pull request for doing that is welcome

coffeemug commented 9 months ago

If anyone finds this in the future, relevant eglot-x PR is https://github.com/nemethf/eglot-x/pull/7. Supports both ccls and clangd (the latter adapted from the eglot manual code pointed out above).