search

enkore / j4-dmenu-desktop

A fast desktop menu
GNU General Public License v3.0
682 stars 69 forks source link

Added a --wrapper option #94

Closed Hritik14 closed 4 years ago

Hritik14 commented 5 years ago

Can execute the commands with a certain wrapper. Helps if you want something like i3 exec $MY_COMMAND and $MY_COMMAND binary is available. Fixes problems like these

Usage: (in i3 context) j4-dmenu-desktop --wrapper "i3 exec"

jdorel commented 5 years ago

Nice !

Although the option could use a more explicit name, like --cmd-prefix.

Hritik14 commented 5 years ago

Check the latest commit. Anything similar to prefix can't be used. Though I could use --cmd-wrapper but that depends on how the maintainer sees fit.

jdorel commented 5 years ago

I do not understand the comment on your commit. When you do:

std::string command = this->wrapper+" \""+get_command()+"\"";

You prefix the command with the wrapper, so wrapper is a prefix, from j4 point of view. Why couldn't you use something similar to prefix ? Can you give me an example so that I understand ?

Hritik14 commented 5 years ago

Wrapper is not just a prefix. It kinds of wraps around the get_command() with double quotes. (Notice the double quotes before and after the user supplied command). This is to ensure that entire user command is sent to wrapper as a single argument even if the user command consists of arguments i.e. is space separated. This is necessary, because by default, i3 exec expects only one argument. If there is more than one, rest are sent to i3 and not i3 exec. An alternative would be --cmd-prefix AND --cmd-suffix but that would needlessly complicate things. The --wrapper is specially built for i3 exec support (and if there's something similar in any other WM). Also, it would be nice to note, that if the user command consists of double quotes then it might cause some complications. But I think that would be a rare case.

jdorel commented 5 years ago

Understood. I am also more in favor of --cmd-wrapper (or the shorter --cmd-wrap). Thanks :)

enkore commented 4 years ago

Thanks, merged it manually.