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

Removed shell-quote-argument for Windows native Emacs to load jar fil… #40

Merged
merged 2 commits into from Nov 11, 2016

Conversation

abcdec
Copy link

@abcdec abcdec commented Oct 22, 2016

…e properly.

shell-quote-argument is preventing native windows Emacs to find jar file. Since start-process is used, it would not need shell quote - should work in Linux as well, but not tested, please test before pull commit.

The original issue was that I was getting

error in process sentinel: if: PLANTUML Preview failed: exited abnormally with code 1
error in process sentinel: PLANTUML Preview failed: exited abnormally with code 1

In *Messages* and
Command is java -jar "c:/Users/tm/AppData/Roaming/.emacs.d/elisp/plantuml.jar"
In *PlantUML Messages*

cygwin Emacs will need another #40 call-process patch to work properly in my environment. Also maybe which version of Java install might make things different.

@skuro
Copy link
Owner

skuro commented Nov 8, 2016

Testing these changes it appears that nothing break on OS X as well as on native Windows emacs. Truth to be told, I could not reproduce your issue neither: up to 68e5e52 it seems to me that native Windows emacs just works.

Also, another question: you say shell-quote-argument breaks in your setup, but that is actually still used in your PR to load the highlighter keywords.

Given the results of my testing it seems like this PR is actually not needed, but maybe you can provide a way to reproduce the issue you are trying to solve?

Thanks

@iquiw
Copy link

iquiw commented Nov 11, 2016

shell-quote-argument (with start-process) causes problem even on Unix when the path contains space.

@skuro
Copy link
Owner

skuro commented Nov 11, 2016

@iquiw interesting, that's one of those cases that shell-quote-argument should be designed to manage, isn't it? Anyway, you are absolutely right, I'll merge the PR asap.

@skuro skuro merged commit 47c7e7c into skuro:develop Nov 11, 2016
@iquiw
Copy link

iquiw commented Nov 11, 2016

@skuro I think shell-quote-argument is designed to be used with functions that execute process via shell, such as start-process-shell-command, shell-command, etc.
(So my comment #14 (comment) might be misleading)
Since start-process does not use shell, it does not require shell-quote-argument.

P.S. I am using plantuml-mode a lot, very useful. Thank you, @skuro and @abcdec.

@skuro
Copy link
Owner

skuro commented Nov 11, 2016

@iquiw I see, I'm still a white belt in emacsland, that's something new I'm learning today :-)
Besides, happy to hear plantuml-mode has been useful!

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.

None yet

3 participants