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.

Sign up for GitHub

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

Jump to bottom

lsp-xml: Allow automatic download of Lemminx (LSP XML) server #2264

Merged
merged 2 commits into from
Oct 21, 2020
Merged

lsp-xml: Allow automatic download of Lemminx (LSP XML) server #2264

merged 2 commits into from
Oct 21, 2020

Conversation

egh
Copy link
Contributor

@egh egh commented Oct 17, 2020 •
edited
Loading

  • Add helper constants to reduce repetition of current version.
  • Store jar in lsp-server-install-dir

@github-actions github-actions bot added the client One or more of lsp-mode language clients label Oct 17, 2020
@egh egh force-pushed the lsp-xml-auto-install-server branch from af00f83 to a47583b Compare October 17, 2020 01:09
kiennq
kiennq reviewed Oct 17, 2020
View reviewed changes
clients/lsp-xml.el Outdated
"org.eclipse.lsp4xml-0.3.0-uber.jar"))
(defconst lsp-xml-jar-version "0.13.1")

(defconst lsp-xml-jar-name (s-concat "org.eclipse.lemminx-" lsp-xml-jar-version "-uber.jar"))
Copy link
Member

Choose a reason for hiding this comment

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

nit: a format would look easier

Suggested change
(defconst lsp-xml-jar-name (s-concat "org.eclipse.lemminx-" lsp-xml-jar-version "-uber.jar"))
(defconst lsp-xml-jar-name (format "org.eclipse.lemminx-%s-uber.jar" lsp-xml-jar-version))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Fixed in 73442c8

kiennq
kiennq reviewed Oct 17, 2020
View reviewed changes
clients/lsp-xml.el Outdated
"Xml server jar command."
:type 'string
:group 'lsp-xml
:type 'file
:package-version '(lsp-mode . "6.1"))

(defcustom lsp-xml-jar-download-url
(s-concat
Copy link
Member

Choose a reason for hiding this comment

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

Same here, format is easier to see how the final url looks like

Copy link
Contributor Author

@egh egh Oct 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the feedback. Fixed in 73442c8

@egh
lsp-xml: Allow automatic download of Lemminx (LSP XML) server
73442c8 
- Add helper constants to reduce repetition of current version.
- Store jar in `lsp-server-install-dir`
@egh egh force-pushed the lsp-xml-auto-install-server branch from a47583b to 73442c8 Compare October 17, 2020 04:16
@kiennq
Copy link
Member

kiennq commented Oct 17, 2020

Just asking but is Lemminx server is newer (maintained) version of lsp4xml?

@egh
Copy link
Contributor Author

egh commented Oct 17, 2020

Just asking but is Lemminx server is newer (maintained) version of lsp4xml?

It seems the project was renamed and transferred to the Eclipse project.
See eclipse-lemminx/lemminx#283 and https://projects.eclipse.org/projects/technology.lemminx
https://github.com/angelozerr/lsp4xml now redirects to https://github.com/eclipse/lemminx

@yyoncho yyoncho merged commit 70877e4 into emacs-lsp:master Oct 21, 2020
@yyoncho
Copy link
Member

yyoncho commented Oct 21, 2020

Thank you!

1 similar comment
@egh
Copy link
Contributor Author

egh commented Oct 21, 2020

Thank you!

@egh egh deleted the lsp-xml-auto-install-server branch October 21, 2020 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kiennq kiennq kiennq left review comments

@yyoncho yyoncho yyoncho approved these changes

Assignees
No one assigned
Labels
client One or more of lsp-mode language clients documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@egh @kiennq @yyoncho