RaiMan / SikuliX-2014

SikuliX version 1.1.2 (until February 2018)
http://sikulix.com
806 stars 234 forks source link

The script runsikulix incorrect on Unix #224

Open UnitedMarsupials-zz opened 8 years ago

UnitedMarsupials-zz commented 8 years ago

The script has several problems:

  1. Although it is delightfully innocent of any bash-isms, it still insists on /bin/bash instead of simply on /bin/sh. While on some OSes/Linux distributions bash is always available -- and is located in /bin -- on others it is an option and/or found elsewhere (such as /usr/local/bin).
  2. It wraps the java-command, but does not use exec to execute it. This often means, the wrapping shell-process hangs around until the wrapped process (java) exits. It also means, that java's exit code is ignored -- shell exits with 0.
  3. It passes its own arguments to the wrapped java-process as $*. This incorrectly expands any parameters with blanks inside them. For example, calling it with two arguments:

    runsikulix -r "My Work/my test"

    will translate into a call with four

    java -jar .../sikulix.jar -r My Work/my test

    and result in error. The correct construct to use is "$@" (note the quotes).

  4. The property sikuli.FromCommandLine does not seem to be accessed anywhere inside SikuliX code...
  5. It "talks too much" -- echo-ing things. Someone debugging a problem with the script would simply add the -x flag to sh, which would cause every command echoed to stderr verbatim. At other times it is simply too much noise. Likewise, the error-message about sikulix.jar "not found" is not necessary -- java would tell the user that same thing itself.
  6. It has inconsistent white-space indentation mixing 2-spaces with tabs.

I'm attaching my own version here as an example. It also provides for invoking a different main-class (IDE or API) depending on how the script itself was invoked. runsikulix

BTW, are there any other useful main-classes (in addition to org.sikuli.ide.Sikulix and org.sikuli.script.Sikulix) which may deserve their own wrapper-scripts?

RaiMan commented 8 years ago

Although it is delightfully innocent of any bash-isms.

I love this remark :-) having German as mother tongue

Great thanks. Much appreciated. I will have a look and try to understand ;-) Anything wrong with putting it into the bundle?

RaiMan commented 8 years ago

BTW, are there any other useful main-classes

The only "real" main-class in fact is org.sikuli.ide.Sikulix.

org.sikuli.script.Sikulix can be used with option -s to start an experimental runserver (not yet in the docs and not complete). You should ignore it ;-)

UnitedMarsupials-zz commented 8 years ago

Anything wrong with putting it into the bundle?

Nothing whatsoever. But it needs to be adjusted for the paths inside. Also, as you can see, I use a separately-installed JXGrabKey.jar (and wish, I could refer to other non-Sikili JARs the same way).