nimaid / LPHK

A Novation Launchpad Macro Scripting System.
GNU General Public License v3.0
336 stars 69 forks source link

CODE fix #112

Closed lorenzgillner closed 2 years ago

lorenzgillner commented 2 years ago

Without the shell=True argument subprocess.run() throws an exception, for example: [Errno 2] No such file or directory: 'notify-send test'

duncte123 commented 2 years ago

What exactly does this argument do? Why is it needed?

lorenzgillner commented 2 years ago

Without this argument, the subprocess call does not resolve the command properly (at least on my system): the command and its arguments are treated as one command, which ofc cannot be found in the PATH. But turns out, that many people consider this option unsafe, so I used a different approach in my recent commit: Previously, args was stored as a string, which led to the incorrect command execution. Now that it's still a list, it works fine and we don't need the shell argument. Users should still be very careful with what they execute, though ;)

lorenzgillner commented 2 years ago

What exactly does this argument do? Why is it needed?

From the subprocess docs (Frequently Used Arguments):

If passing a single string, either shell must be True (see below) or else the string must simply name the program to be executed without specifying any arguments.

... and that's currently the case. If I want to execute a script or command line program with a few parameters using the CODE command, the execution fails.

Also:

If shell is True, the specified command will be executed through the shell. This can be useful if you are using Python primarily for the enhanced control flow it offers over most system shells and still want convenient access to other shell features such as shell pipes, filename wildcards, environment variable expansion, and expansion of ~ to a user’s home directory.

... desirable features, imo.

duncte123 commented 2 years ago

If that's the case I'm fine with it being set to True

lorenzgillner commented 2 years ago

Okay, I changed it back to True. I tried to apply the same changes to CODE_NOWAIT, but because of the syntax checking for that command, it is not functioning as intended. Gonna investigate ...