Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added --run-command option, obsoletes --wrapper #104

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/Main.hh
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ private:
"\tIn this mode entries are sorted by usage frequency.\n"
" --wrapper=<wrapper>\n"
"\tA wrapper binary. Useful in case you want to wrap into 'i3 exec'\n"
" --run-command=<cmd>\n"
"\tThe command to run instead of the selected binary. You can embed the"
" selected binary name using `{}` keyword"
" --wait-on=<path>\n"
"\tMust point to a path where a file can be created.\n"
"\tIn this mode no menu will be shown. Instead the program waits for <path>\n"
Expand Down Expand Up @@ -137,6 +140,7 @@ private:
{"wait-on", required_argument, 0, 'w'},
{"no-exec", no_argument, 0, 'e'},
{"wrapper", required_argument, 0, 'W'},
{"run-command", required_argument, 0, 'r'},
{0, 0, 0, 0}
};

Expand Down Expand Up @@ -175,6 +179,9 @@ private:
case 'W':
this->wrapper = optarg;
break;
case 'r':
this->run_command = optarg;
break;
default:
exit(1);
}
Expand Down Expand Up @@ -256,6 +263,12 @@ private:
std::string command = get_command();
if (this->wrapper.length())
command = this->wrapper+" \""+command+"\"";

// --run-command switch
if (this->run_command.length()){
command = replace(this->run_command, this->run_command_token, command);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think command should be shell-quoted here, so that the command always gets the command line as a single argument? Not sure, really.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that totally depends on the end user. They can perfectly choose to shell quote the {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One the other hand, if we do shell-quote by default and the user then - by standard practice - quotes again, it would be something like "{}" -> ""cmd"" which would completely defy the quotes. I guess the current approach should suffice. Let me know what you think.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only familiar with the {} syntax from find(1) - there are likely others, which probably behave differently - which escapes the file name (e.g .find -exec strace -e execve echo {} \; with some files with spaces in their names will look like this: execve("/usr/bin/echo", ["echo", "./foo bar baz"], ..., so the {} becomes a single argument).

Copy link
Contributor Author

@Hritik14 Hritik14 Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new commit to resolve this issue. This is as far I could think which would maintain $SHELL usage and won't break with weird filenames (ones with quotes in them)


delete this->dmenu;

if(!command.empty()) {
Expand Down Expand Up @@ -346,6 +359,9 @@ private:
std::string dmenu_command;
std::string terminal;
std::string wrapper;
std::string run_command;

const std::string run_command_token = "{}";
const char *wait_on = 0;

stringlist_t environment;
Expand All @@ -369,4 +385,3 @@ private:

const char *usage_log = 0;
};