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

Extend lsp-mode to provide option to register action handlers #328

Merged
merged 3 commits into from
Apr 25, 2018

Conversation

yyoncho
Copy link
Member

@yyoncho yyoncho commented Apr 1, 2018

Fixes #309

@vibhavp
Copy link
Member

vibhavp commented Apr 3, 2018

IMO, url handlers can be handled/registered better by a LSP client in the lsp-define-client macros.

@yyoncho
Copy link
Member Author

yyoncho commented Apr 3, 2018

Hi @vibhavp ,

Thanks for the feedback!

I am a bit confused, I tried to follow the patters established by the lsp-mode, e. g. both notification-handlers and request-handlers have corresponding register method and I tried to make it as consistent as possible. Are you planning to move both to the define client macros as well?

@yyoncho
Copy link
Member Author

yyoncho commented Apr 4, 2018

@vibhavp Now I get it - I believe your comment is not for this request but for #326 ? I was considering moving the url handlers in the lsp--workspace but I wasn't sure what was the lifetime of the client. I will try to move it.

@vibhavp
Copy link
Member

vibhavp commented Apr 4, 2018

Yeah, this was on the incorrect issue. Regardless, does the specification allow code actions to be executed by the client?

@yyoncho
Copy link
Member Author

yyoncho commented Apr 4, 2018

From the spec:

"The protocol currently doesn’t specify a set of well-known commands. So executing a command requires some tool extension code."

lsp-methods.el Outdated
;; can be used in `lsp-execute-code-action' to determine whether the action
;; current client is interested in executing the action instead of sending it
;; to the server.
(action-handlers (make-hash-table :test 'equal) :read-only t)
Copy link
Member

Choose a reason for hiding this comment

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

Could you make this the last field of the struct? Otherwise, we'd be unnecessarily breaking binary compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I didn't know that this will have an effect on binary compatibility, thanks!

Copy link
Member Author

@yyoncho yyoncho Apr 9, 2018

Choose a reason for hiding this comment

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

Comment addressed with 8f07c98

@vibhavp
Copy link
Member

vibhavp commented Apr 25, 2018

Merging for now, as all documentation for code actions I could find is still ambiguous.

@vibhavp vibhavp merged commit f982fee into emacs-lsp:master Apr 25, 2018
@yyoncho yyoncho deleted the action-handlers branch December 14, 2018 09:20
wkirschbaum pushed a commit to wkirschbaum/lsp-mode that referenced this pull request Jun 1, 2021
…#331)

* Add +swbt none to remove busy-wait and improve performance

Utilizing busy-wait to reduce latency primarily makes sense for
applications that are running as the only node on the machine. So it
doesn't make sense for ElixirLS because there will usually be other
processes running on the machine such as the editor and potentially
other ElixirLS instances.

A blog post that touches on `+swbt none`:
https://www.ably.io/blog/beam-optimization-mqtt/

Also add sbwtdcpu and sbwtdio

Fixes emacs-lsp#328

* Add +sbwt none to .bat files also

* Update changelog
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