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

Support for 'xml/executeClientCommand` access to server from extension #905

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

BoykoAlex
Copy link
Contributor

@BoykoAlex BoykoAlex commented Oct 8, 2020

Spring XML Extension needs two things from the XML server:

  • Support for xml/executeClientCommand to execute a command on the client. The client command execution would first try to find a command registered by an LS if not found would attempt to execute a client command (registered command in Eclipse or in VSCode)
  • Access to XMLLanguageServer instance to register/unregister command and send xml/executeClientCommand request to the client. Need to work out nice API around it as it looks like XMLLanguageServer or XMLLanguageClient was not meant to be accessed by extensions.

Eclipse Client PR: eclipse-wildwebdeveloper/wildwebdeveloper#543

@fbricon
Copy link
Contributor

fbricon commented Oct 8, 2020

@BoykoAlex thanks for your PR. However, before your contribution can be accepted by the project, you need to create and electronically sign an Eclipse Contributor Agreement (ECA):

  1. Log in to the Eclipse foundation website. You will need to
    create an account with the Eclipse Foundation if you have not already done so.
  2. Click on "Eclipse ECA", and complete the form.

Be sure to use the same email address in your Eclipse account that you intend to use when you commit to GitHub.
All committers all commits are bound to the Developer Certificate of Origin.
As such, all parties involved in a contribution must have valid ECAs and commits must include valid "Signed-off-by" entries.
Commits can be signed off by including the -s attribute in your commit message, for example, git commit -s -m 'Interesting Commit Message.

@fbricon fbricon added the enhancement New feature or request label Oct 8, 2020
@BoykoAlex
Copy link
Contributor Author

@angelozerr The code is ready for the second code review pass :-) There is also #906 factored out from this PR.

@angelozerr
Copy link
Contributor

angelozerr commented Oct 9, 2020

@angelozerr The code is ready for the second code review pass :-)

Let me review more but I have read quickly your code and it's seems great.

It should be nice if you could:

For each service it should be nice to give a description and a sample.

There is also #906 factored out from this PR.

Yes I have seen that, thanks.

@angelozerr
Copy link
Contributor

@BoykoAlex we would like to do a release of vscode-xml in the following weeks. Have you planned to work on this issue soon?

@BoykoAlex
Copy link
Contributor Author

@angelozerr VSCode PR: redhat-developer/vscode-xml#339

@BoykoAlex BoykoAlex force-pushed the spring-nsuri branch 2 times, most recently from 152cd1e to c06a8c0 Compare October 9, 2020 23:36
@BoykoAlex
Copy link
Contributor Author

@angelozerr Looks like aside from the opened question of switching xml/executeClientCommand' to workspace/executeClientCommand` everything is addressed and ready to be re-evaluated.
(The vscode PR would be affected by the answer to the question above too)

@angelozerr
Copy link
Contributor

@angelozerr Looks like aside from the opened question of switching xml/executeClientCommand' to workspace/executeClientCommand`

Please wait feedback from @fbricon .

everything is addressed and ready to be re-evaluated.

Your PR seems very great (API, quality of your code, comments, etc), but I have not tested yet.

(The vscode PR would be affected by the answer to the question above too)

That's great, thank a lot, I will review it ASAP.

@angelozerr
Copy link
Contributor

angelozerr commented Oct 11, 2020

@angelozerr
Copy link
Contributor

@BoykoAlex your PR looks great, let me try to implement a custom command to check your PR.

@angelozerr
Copy link
Contributor

@BoykoAlex could you fix please #905 (comment)

@angelozerr angelozerr merged commit 11f6f6d into eclipse-lemminx:master Oct 13, 2020
@angelozerr
Copy link
Contributor

@BoykoAlex thanks so much for your great PR and your patience! Awesome!

@BoykoAlex
Copy link
Contributor Author

BoykoAlex commented Oct 13, 2020

Thanks for getting this is in @angelozerr and for the thorough review :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants