Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
chrisbouchard opened this issue Apr 16, 2024 · 0 comments
Open

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

chrisbouchard opened this issue Apr 16, 2024 · 0 comments

Comments

@chrisbouchard
Copy link

chrisbouchard commented Apr 16, 2024

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.

@chrisbouchard chrisbouchard changed the title lsp-interface generates a lot of noise in pcase lsp-interface generates a lot of noise in pcase documentation Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant