JonnyHaystack / i3-resurrect

Simple solution to saving and restoring i3 workspaces
GNU General Public License v3.0
379 stars 19 forks source link

Fix multi arg commands #50

Closed nnist closed 5 years ago

nnist commented 5 years ago

Thanks for making this project! I've been playing around with this for a bit, and it seems to work great, although I ran into a small issue. When custom commands with multiple arguments are defined in *_programs.json, they don't get executed. For example:

[
  {
    "command": [
      "alacritty -d 0 0 -e '.config/i3/finances/vim-beancount.sh' &"
    ],
    "working_directory": "/home/nicole"
  },
  {
    "command": [
      "firejail --overlay-tmpfs --ignore=apparmor firefox --no-remote"
    ],
    "working_directory": "/home/nicole"
  },
  {
    "command": [
      "alacritty"
    ],
    "working_directory": "/home/nicole"
  }
]

With the above settings only the last command is executed, the rest silently fails without any errors.

This tiny PR fixes that by removing quotes in the cmdline parsing. The commands are run as expected now. It also adds an extra check on the reponse i3 gives back. This could probably be more elegant, but it is helpful in debugging when creating custom commands.

JonnyHaystack commented 5 years ago

Hi, thanks for bringing it to my attention that you're having trouble with this.

The main problem here is that it's expected that each argument is a separate element in the command array. This worked great when I was using Python's subprocess module, because that also expects a list of arguments. However, with i3's exec command, you need to pass the command as one string.

This is where the problem starts. The cmdline that you get from the process is an array, which is the best because you don't have to worry about quoting arguments that may contain spaces (e.g. file paths, arbitrary strings). But when you pass the command to i3's exec, you need to join the command into one string. Because the original cmdline was an array, you don't know which arguments NEED quotes. To solve this, I just wrapped all arguments in quotes, because that makes i3 exec happy.

So now you understand why I must wrap the arguments in quotes, otherwise only single argument commands are guaranteed to work correctly.

The reason you're having this problem is because you are trying to put multiple arguments into one element of the cmdline array. This means the whole command is wrapped in quotes, which in turn means i3's exec command thinks the whole thing is one argument when it should actually just be the name of an executable.

So to solve your problem, simply break up the arguments of your custom commands into separate array elements like so:

"command": [
    "alacritty",
    "-d",
    "0",
    "0",
    "-e",
    ".config/i3/finances/vim-beancount.sh",
    "&"
],

That might need a little bit of tweaking as I cannot test with a script that I don't have, but you get the idea.

Alternatively it would probably work to just not use a list and simply put:

"command": "alacritty -d 0 0 -e '.config/i3/finances/vim-beancount.sh' &",

Anyway yeah it's very messy using i3 exec and I hate it.. Using subprocess was much cleaner but I switched to fix issue #40

If you can find some other viable fix for #40 I'd ditch i3 exec without hesitation.

nnist commented 5 years ago

Thanks for the detailed response! I tested both of your suggestions and they both work, so I think this was just an user error on my part... Though, in case you're sticking with the i3 exec solution it might be helpful for others if this is added to the readme.

I'll close this PR for now, and will look into an alternative fix if I can find some time for it.