search

eclipse / shellwax

Shell script editor plugin for Eclipse
https://marketplace.eclipse.org/content/shellwax
Eclipse Public License 2.0
17 stars 12 forks source link

bash language server installation fails on eclipse 2020-03 on windows 10 #38

Closed level420 closed 4 years ago

level420 commented 4 years ago

Environment:

After installing shellwax from within the eclipse marketplace window and opening a .sh file, the progress window displays on each click and edit attempt the following message:

Bash language server installation

The cause is that method isInstalled here https://github.com/eclipse/shellwax/blob/master/org.eclipse.shellwax.core/src/org/eclipse/shellwax/internal/BashLanguageServer.java#L37 searches at the wrong path.

In fact the bash-language-server executable which is bash-language-server.cmd on windows is installed in the path:

%HOMEPATH%\.local\share\shellwax\1.11.1\bash-language-server.cmd

but method isInstalled tests at the location

%HOMEPATH%\.local\share\shellwax\1.11.1\node_modules\.bin\bash-language-server.cmd

Manually creating the directory %HOMEPATH%\.local\share\shellwax\1.11.1\node_modules\.bin\ and copy bash-language-server.cmd stops shellwax from displaying the message Bash language server installation, but I don't know if this solves the problem.

level420 commented 4 years ago

One addition: to really stop the repeatedly bash language server installation messages, I also had to create a %HOMEPATH%\.local\share\shellwax\1.11.1\node_modules\.bin\bash-language-server file, to overcome the test for the file existence.

akurtakov commented 4 years ago

I don't have Windows machine to test with. In the early days of the project I borrowed windows machine to verify things. What do you think about putting if (Platform.getOS().equals(Platform.OS_WIN32)) { in isInstalled making it check the correct places. I ask you to do it cause you can verify it directly rather than going through some untested patches coming from my side. If you hit any issue trying to do it let me know.

level420 commented 4 years ago

@akurtakov I've tried a PR (see #39) without having the possibility to test it here because I don't have the necessary java environment here. But you could create a plugin somewhere where I can pick it up and install and test it here. And you could test it on linux/rhel etc.

level420 commented 4 years ago

@akurtakov would it be possible to install the plugin in eclipse using the files from eclipse jenkins somehow? I mean from here:

https://ci.eclipse.org/shellwax/job/shellwax/job/PR-39/3/

level420 commented 4 years ago

Oh yes, this works when using the path https://ci.eclipse.org/shellwax/job/shellwax/job/PR-39/3/artifact/org.eclipse.shellwax.site/target/repository/ as an installation resource! And the PR works for me!

akurtakov commented 4 years ago

Cool. would you please sign Eclipse Contributor Agreement https://www.eclipse.org/legal/ECA.php so I can accept your patch. Merging all the changes in single commit will make it easier to read too.

level420 commented 4 years ago

OK. I've signed the ECA.

When merging the PR you could choose "squash merge" which compresses all commits into a single commit.

level420 commented 4 years ago

I've closed #39 and opened #40 to include the Signed-off-by thing to make the eca-test happy. And it is a single commit.

akurtakov commented 4 years ago

Build available at https://download.eclipse.org/shellwax/snapshots/plugins/

level420 commented 4 years ago

@akurtakov does this mean that the fixed version is available on the eclipse marketplace?

akurtakov commented 4 years ago

Yes

level420 commented 4 years ago

Installed and it worked! Great!