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

Shell processes left in the background #135

Closed
cassie-is-asleep opened this issue Nov 24, 2022 · 2 comments
Closed

Shell processes left in the background #135

cassie-is-asleep opened this issue Nov 24, 2022 · 2 comments

Comments

@cassie-is-asleep
Copy link

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
Copy link
Author

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 added a commit to meator/j4-dmenu-desktop that referenced this issue Nov 26, 2022
@meator
Copy link
Collaborator

meator commented Nov 26, 2022

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.

meator added a commit to meator/j4-dmenu-desktop that referenced this issue Aug 2, 2023
meator added a commit to meator/j4-dmenu-desktop that referenced this issue Aug 2, 2023
@meator meator closed this as completed in a8ec991 Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants