enkore / j4-dmenu-desktop

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

Shell processes left in the background #135

Closed cassie-is-asleep closed 1 year ago

cassie-is-asleep commented 1 year ago

It appears that when an application is selected and a command is passed to the shell, the shell remains open until the application exits. This is of very little consequence, but is something that can be done more neatly and correctly.

I believe the change is as simple as prefixing "exec", followed by a space, to the shell command right before it is executed, causing the shell to exit while it launches the command. Right about here is where I think the change could easily be made, but embarassingly I have zero prior experience with C++ as compared to standard C and posix shell scripting.

Thanks for your time!

cassie-is-asleep commented 1 year ago

In the meantime, here's a quick POSIX sh script that does such a fix externally, allowing you to quickly test the impact such a change would have.

#!/bin/sh
printf 'exec %s\n' "$(j4-dmenu-desktop --no-exec "$@")" | ${SHELL:-sh} &
meator commented 1 year ago

This is a interesting problem. J4dd executes the programs with <shell> -c <command_string>. Some shells (like bash) automatically exec the last component of the command string (in most cases the command string has only one component in j4dd so the program is always execed). But some shells don't do that (like dash).

I think this is why no one (including me) has discovered this issue.

I've incorporated a fix into my PR #132. I would recommend you to compile and use the PR's version of j4dd because it contains many fixes and new features.

If don't want to use my version of j4dd you can apply this patch to j4dd:

--- a/src/Main.hh
+++ b/src/Main.hh
@@ -260,7 +260,9 @@ private:
         }

         this->dmenu->display();
-        std::string command = get_command();
+        std::string command;
+        bool iscustom;
+        std::tie(command, iscustom) = get_command();
         if (this->wrapper.length())
             command = this->wrapper+" \""+command+"\"";
         delete this->dmenu;
@@ -273,6 +275,8 @@ private:
             static const char *shell = 0;
             if((shell = getenv("SHELL")) == 0)
                 shell = "/bin/sh";
+            if (!iscustom)
+                command = "exec " + command;

             fprintf(stderr, "%s -c '%s'\n", shell, command.c_str());

@@ -317,7 +321,7 @@ private:
         return 0;
     }

-    std::string get_command() {
+    std::pair<std::string, bool> get_command() {
         std::string choice;
         std::string args;
         Application *app;
@@ -326,13 +330,13 @@ private:

         choice = dmenu->read_choice(); // Blocks
         if(choice.empty())
-            return "";
+            return std::make_pair("", false);

         fprintf(stderr, "User input is: %s %s\n", choice.c_str(), args.c_str());

         std::tie(app, args) = apps.search(choice, exclude_generic);
         if (!app) {
-            return args;
+            return std::make_pair(args, true);
         }

         if(usage_log) {
@@ -346,7 +350,7 @@ private:
         }

         ApplicationRunner app_runner(terminal, *app, args);
-        return app_runner.command();
+        return std::make_pair(app_runner.command(), false);
     }

 private:

My code (both the patch and the fdb58ae commit in #132) only prepends "exec " to programs which have a desktop file. I don't know if you knew this (it isn't documented anywhere as far as I know and I discovered it only after thoroughly reading the code) but when a command that doesn't match any desktop entry is entered, j4dd tries to execute it directly. Because you can provide custom commands to j4dd, prepending "exec " to them might produce unexpected results.