emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.72k stars 861 forks source link

lsp-interface generates a lot of noise in pcase documentation #4430

Open chrisbouchard opened 2 months ago

chrisbouchard commented 2 months ago

Is your feature related or already mentioned on the wishlist? — No, I don't believe so.

TL;DR

I'd like to consider if there's a way lsp-mode could be (or could evolve to be) more conscientious with its use of pcase-defmacro.

Context

I recently opened the function documentation for pcase in my Emacs (to read about the map pattern). I noticed that, interspersed with the standard documentation, there was a lot of “noise” like:

-- (WatchKind &rest PROPERTY-BINDINGS)

Not documented.

-- (CodeAction &rest PROPERTY-BINDINGS)

Not documented.

-- (JSONError &rest PROPERTY-BINDINGS)

Not documented.

To quantify, the function documentation for `pcase' in my Emacs is just under 1300 lines long, of which about 1200 are these auto-generated patterns.

I didn't know where these patterns came from at first, but a quick Google for “emacs WatchKind” lead me to lsp-protocol.el, and from there I found the pcase-macro in lsp-interface (added in #2481). pcase-defmacro includes each new pattern into pcase's function documentation for discoverability.

As a user, this is not a great experience. It's important to me that the pcase function documentation be readable, because it's the primary in-editor way that I can discover what patterns are available (including via extensions). I do appreciate that these are technically patterns that are available, but I get the sense they weren't really intended for general consumption, and anyway their sheer number overwhelms any usefulness in documenting them like this.

Also, since these patterns aren't namespaced, there's (a) no indication that they belong to this project, and (b) no guarantee they don't collide with patterns defined by a different package.

My Ideal Solution

Forgetting for a moment that there is already code using the existing patterns, I would love if lsp-interface followed the approach used by cl-struct and defined a single pcase pattern like (lsp-interface INTERFACE &rest PROPERTY-BINDINGS). This pcase-defmacro could include a proper docstring—e.g., describing its use and the forms allowed as property bindings. It would also address the namespacing concern. And most importantly (for me), it would be a single entry in the pcase function documentation rather than hundreds.

I imagine that the current generated pcase-defmacro in lsp-interface could be repurposed as a generated lsp-interface--pcase-pattern-INTERFACE function, to which the lsp-interface pattern could delegate based on its interface parameter. (This would happen during macro compilation, so there wouldn't be a runtime cost for the indirection.)

So for example, code like

(pcase expr
  ((JSONError :message :code) ...))

would instead be written

(pcase expr
  ((lsp-interface JSONError :message :code) ...))

and expand to the exact same base pcase patterns.

Something Practical?

Returning to the real world, I appreciate that my suggestion would be a breaking change. I suppose the first question is: Does my suggestion seem palatable? My primary goal is to keep the pcase function documentation readable, so I'm not married to it in particular—I'm just not sure there's a solution other than to reduce the number of patterns defined. (For example, I think it would be a non-starter to simply include a docstring in the existing pcase-defmacro, since that would add hundreds more lines of repeated documentation to pcase's documentation.)

Also, as a newcomer to lsp-mode's development, are these interface patterns intended to be used outside of lsp-mode itself? I did a quick GitHub code search and only found a couple uses in lsp-mode itself. I also couldn't find the feature mentioned in the changelog or documentation. If this isn't a supported feature, maybe a breaking change wouldn't be too painful?

Alternatively, I imagine a new lsp-interface pattern could be added along side the existing patterns, which could then be deprecated. For example, the existing pcase-defmacro body could be turned into the generated lsp-interface--pcase-pattern-INTERFACE function, and the pcase-defmacro repurposed to return a new lsp-interface pattern.

Thanks for your attention and feedback. Whatever the path forward, I'll be happy to contribute—I don't mean this to be a rant or demand! I'll probably start poking at the deprecation approach when I get some free time.