Skip to content

Support client capabilities (ocaml.next-hole) #54

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

Merged

Conversation

xvw
Copy link
Member

@xvw xvw commented Apr 15, 2025

No description provided.

@xvw xvw requested a review from voodoos April 15, 2025 13:52
Copy link
Member

@voodoos voodoos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's out of my league :-)

Are the test failures expected ?

@xvw xvw force-pushed the let-s-try-to-support-client-capabilities-first-experiment branch 2 times, most recently from 5a47175 to 1125507 Compare April 15, 2025 16:07
@xvw xvw force-pushed the let-s-try-to-support-client-capabilities-first-experiment branch 3 times, most recently from c4c98e0 to 67654b1 Compare April 15, 2025 17:47
@xvw xvw force-pushed the let-s-try-to-support-client-capabilities-first-experiment branch 2 times, most recently from 1029262 to e58ae4a Compare April 15, 2025 18:56
@xvw xvw force-pushed the let-s-try-to-support-client-capabilities-first-experiment branch from e58ae4a to 1e9976a Compare April 15, 2025 18:57
ocaml-eglot.el Outdated
Comment on lines 712 to 726
(when (fboundp 'eglot-execute)
(cl-defmethod eglot-execute :around (_ action)
"Custom handler for performing client commands."
(pcase (cl-getf action :command)
("ocaml.next-hole" (ocaml-eglot--command-next-hole
(cl-getf action :arguments)))
(_ (cl-call-next-method)))))

(when (fboundp 'eglot-execute-command)
(cl-defmethod eglot-execute-command :around (_ command arguments)
"Custom handler for performing client commands (legacy)."
(pcase command
("ocaml.next-hole" (ocaml-eglot--command-next-hole arguments))
(_ (cl-call-next-method)))))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be useful for later or other dev to comment here on the reason why you need both of these.
In general these extensions looks quite tricky, documenting your implementation choices and the difficulty you encountered would be very valuable.

@voodoos voodoos merged commit 3d4a1f3 into main Apr 15, 2025
5 checks passed
@xvw xvw deleted the let-s-try-to-support-client-capabilities-first-experiment branch April 15, 2025 19:50
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

Successfully merging this pull request may close these issues.

2 participants