asterics / AsTeRICS

The Assistive Technology Rapid Integration & Construction Set
http://www.asterics.eu
Other
57 stars 27 forks source link

Klaus/issue#269/f8 opens explorer #270

Closed klues closed 5 years ago

klues commented 5 years ago

My solution for #269 although I'm not sure if it is perfect. It resolves the main problem -> F8 now again opens WebACS (tested on Windows and Linux)

But the general problem in the ApplicationLauncher plugin was: sometimes we need quotes on arguments, sometimes not. More exactly: 1) OPEN_URL mode with argument like "http://www.google.com/search?source=hp&btnK=Google-Suche&q=test" needs the quotes on windows because otherwise the special cmd-char & breaks the command 2) START_APPLICATION mode with e.g. application xterm and arguments -e "sudo sh test.sh" need to be parsed to the arguments -e and sudo sh test.sh without quotes on the last part, otherwise it does not work (tested on ubuntu)

Therefore my solution is the following: 1) I introduced a new private method where the array of command+argument can be directly passed. openURL() uses it and therefore can pass the URL with qoutes as needed for problem (1) 2) the normal startApplication() again removes the qoutes so that problem case (2) works correclty.

I've tested several scenarios on Mac, Windows and Linux: 1) Open URLs like www.google.com/search?source=hp&btnK=Google-Suche&q=test, "www.google.com/search?source=hp&btnK=Google-Suche&q=test", google.at or "google.at" --> all of this works! 2) Start commands like xterm -e "sudo sh test.sh", ps, ps -aux, ls / (on linux) or dir, explorer <path with spaces> --> all of this works!

So it seems that it should be a good solution. However it is not possible to use START_APPLICATION where a quoted parameter remains quoted in the final string-array. I don't know if this is needed at any point.

deinhofer commented 5 years ago

Regarding point 2) startApplication:

Are you sure that xterm -e "sudo sh test.sh"

does not run on Linux? I just tested it on my Ubunutu and it seemed to work. What commands did you use within test.sh?

klues commented 5 years ago

yeah, on console xterm -e "sudo sh test.sh" works, you even have to write it that way. But with the process builder in Java passing the Array ["xterm", "-e", "\"sudo sh test.sh\""] does not work but it has to be ["xterm", "-e", "sudo sh test.sh"]. I tested this on Ubuntu.

deinhofer commented 5 years ago

Ok, I have done some more in depth tests using a simple program that allows to test commands and arguments on certain platforms. TestProcessBuilder.zip

I have found out that the real problem on windows is not windows in general but the & (and maybe other special characters of cmd.exe) which is treated as line break for arguments: See https://stackoverflow.com/questions/8055371/how-do-i-run-two-commands-in-one-line-in-windows-cmd

So for example if you use firefox or Internet Explorer to start the given URL it works without quoting the URL.

So, what if we delegate the problem to areProperties where the user can configure the start command per platform? We could change the syntax to e.g.

ARE.openURL.cmd.linux=sensible-browser %1$s 
ARE.openURL.cmd.windows=explorer "%1$s"
ARE.openURL.cmd.linux=sensible-browser %1$s 
ARE.openURL.cmd.windows=firefox %1$s

and than use https://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html to create a formatted string?

Then the treatment of command arguments and URL arguments should always be the same.

Other useful links about quoting for cmd.exe and starting commands on certain platforms: Escaping in cmd.exe Open file URLs from command line

deinhofer commented 5 years ago

Ok, let's keep the current implementation. Please attach test models to make tests objective.

Please update the plugin documentation that

klues commented 5 years ago

added testcases in comment of issue #269

klues commented 5 years ago

updated documentation. I think (and hope) we are ready to merge now? @deinhofer do you want to take a final look, or should I just merge?

deinhofer commented 5 years ago

great contribution, thanks to @klues