Skip to content
This repository was archived by the owner on Jul 31, 2023. It is now read-only.

Setup MS PYLS automatically. #37

Merged
merged 3 commits into from
Jun 30, 2019
Merged

Setup MS PYLS automatically. #37

merged 3 commits into from
Jun 30, 2019

Conversation

seagle0128
Copy link
Collaborator

Close #33.

Get the lastest version of MS PYLS from the Microsoft website, and download to the specified path,
then setup to lsp automatically. Additional setup is not necessary. It's out of box.

Close #33.

Get the lastest version of MS PYLS from the Microsoft website, and download to the specified path,
then setup to lsp automatically. Additional setup is not necessary. It's out of box.
Copy link
Member

@yyoncho yyoncho left a comment

Choose a reason for hiding this comment

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

Looks good! Few minor comments which may or may not be addressed as part of this PR.

((executable-find "powershell")
"powershell -noprofile -noninteractive \
-nologo -ex bypass Expand-Archive -path '%s' -dest '%s'")
(t nil))))
Copy link
Member

Choose a reason for hiding this comment

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

We may consider asking the user to manually unzip the file in case there is no powershell on this PC

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The powershell is built-in since Windows 7. And the behavior is same as dap-mode (dap-utils.el).

Copy link
Member

Choose a reason for hiding this comment

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

Then it seems to be ok to rely on that, thank you.

@smallzhan
Copy link
Contributor

Nice work! A minor issue is that, on windows system, when mspyls is in use, (lsp-pyton-ms-update-server) may fail because of the mspyls directory can't be deleted ("Permission denied"), it will only success when mypyls is not started or killed by hand. That is a question only in windows system, i.e. if a file is used by some process, the directory containing that file can't be renamed or deleted...

If keep the updated server in a new directory, there must be a file to keep track the latest version, make the extension more complicated... So, maybe some instructions in source file is a good choice.

@seagle0128
Copy link
Collaborator Author

seagle0128 commented Jun 30, 2019

@smallzhan Good point! But it's not necessary to handle this scenario. I will add doc strings later.

@seagle0128
Copy link
Collaborator Author

@yyoncho I think it's fine to merge now.

@yyoncho yyoncho merged commit e987d3d into emacs-lsp:master Jun 30, 2019
@twlz0ne
Copy link

twlz0ne commented Jun 30, 2019

I think async-shell-command is better than shell-command.

Also, is there a curl in powershell?

@yyoncho
Copy link
Member

yyoncho commented Jun 30, 2019

I think async-shell-command is better than shell-comman

In general yes, but lsp-mode is expecting the command to finish synchronously. Until this is fixed we might introduce an interactive async version of the installation command.

Also, is there a curl in powershell?

AFAIK no.

@cireu
Copy link

cireu commented Jun 30, 2019

Windows 10 contains a limited curl, but hard to use (I means it can't work well with request). @twlz0ne @yyoncho

@MikusR
Copy link

MikusR commented Jul 5, 2019

Windows 10 contains a limited curl, but hard to use (I means it can't work well with request). @twlz0ne @yyoncho

Windows 10 1903 has full curl 7.55.1. When called from powershell you have to use curl.exe as simply curl is an alias to Invoke-WebRequest.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Setup server automatically
6 participants