[BUG] Can't run shell commands in drun mode

[Myvillithdar]

Version

Output of rofi -v
Version: 1.6.0

Configuration

output of rofi -help:
https://gist.github.com/Myvillithdar/3f2a4666ef96b350092c9c2992577960

my config file:
https://gitlab.com/Myvillithdar/i3config/-/blob/master/base-files/config.rasi

Launch Command

rofi -show drun

Steps to reproduce

• Type reboot
• Press enter

What behaviour you see

• Nothing happens

What behaviour you expect to see

• Before 1.6.0, my computer would reboot (b/c reboot doesn't match any desktop files, rofi would run reboot from /usr/bin/, basically falling back to run mode behavior.)

Additional details:

Under "drun", the manpage says:

Pressing the accept-custom binding (control-enter or shift-enter) with custom input (no entry matching) will run the command in a terminal.

...but control-enter and shift-enter also don't do anything.

I see that the changelog for 1.6.0 includes:

[DRun] Don't run custom commands.

I'm not entirely sure if this is what that refers to, so maybe this was an intentional change, but in any case I'd expect control-enter or shift-enter to still run the command since that's what the manpage says. If this was an intentional change, then it would be nice to have a config option to restore the old behavior.

[mkhan45]

If this was an intentional change, then it would be nice to have a config option to restore the old behavior.

I agree; it's kind of essential to my workflow.

Regardless, the man page claims that pressing the accept-custom binding in drun will run a custom command and that's not the case right now.

[DaveDavenport]

Original issue: #966

[mkhan45]

Do you intend to restore the previous behavior in any way or should I fork the repo and be out of your way?

[DaveDavenport]

please fork the repo.

[bruno-unna]

@mkhan45 Just to understand the issue, what is wrong with using the "run" mode in addition to "drun"?

[mkhan45]

@mkhan45 Just to understand the issue, what is wrong with using the "run" mode in addition to "drun"?

@bruno-unna Mostly because I don't want to have to use different keybindings for starting programs and running commands, but also because I don't want to have to press ctrl+enter instead of enter to run a shell command. Additionally, I'd never use the run mode for a purpose other than running shell commands, so it's just more convenient to have it packed into drun.

[Myvillithdar]

Hmmm, I definitely see the point in #966 -- that error message after a typo is inconvenient to deal with. In my mind the ideal solution would be to add a config option for drun mode that runs the input as a shell command if the list is empty and if a matching command exists in $PATH, and does nothing otherwise.

I suppose as a workaround, I could simply create desktop entries for shutdown and reboot, and that would be enough to largely fix my own workflow (though I can't speak for anyone else). I've always appreciated the convenience of being able to invoke arbitrary commands from drun mode, but at the moment I can't think of any other commands that I regularly do that with.

[DaveDavenport]

The problem is a bit that there is no one solution to please everybody, but you cannot add 1001+ options either.
I myself use combi mode to combine window/drun/run, for me that avoids the need to run custom commands. If I still need to, I can do !r command to run an custom command.

[DaveDavenport]

I guess we need to come up with some solution, (beside forking)

[DaveDavenport]

I kinda like the idea of returning to the list on error. @dannycolin would that work for you?

[dannycolin]

I kinda like the idea of returning to the list on error. @dannycolin would that work for you?

Yes. It would.

[DaveDavenport]

A quick test, seems that I had a lot of infra already there 😄

diff --git a/include/view.h b/include/view.h
index f5deac96..5268f7d8 100644
--- a/include/view.h
+++ b/include/view.h
@@ -193,6 +193,7 @@ RofiViewState * rofi_view_get_active ( void );
  *
  */
 void rofi_view_set_active ( RofiViewState *state );
+void rofi_view_remove_active ( RofiViewState *state );
 
 /**
  * @param msg The error message to show.
diff --git a/source/dialogs/drun.c b/source/dialogs/drun.c
index aac7a934..94200cbc 100644
--- a/source/dialogs/drun.c
+++ b/source/dialogs/drun.c
@@ -1041,7 +1041,12 @@ static ModeMode drun_mode_result ( Mode *sw, int mretv, char **input, unsigned i
         }
     }
     else if ( ( mretv & MENU_CUSTOM_INPUT ) && *input != NULL && *input[0] != '\0' ) {
-        retv = RELOAD_DIALOG;
+        RofiHelperExecuteContext context = { .name = NULL };
+        gboolean             run_in_term = ( ( mretv & MENU_CUSTOM_ACTION ) == MENU_CUSTOM_ACTION );
+        // FIXME: We assume startup notification in terminals, not in others
+        if ( ! helper_execute_command ( NULL, *input, run_in_term, run_in_term ? &context : NULL ) ) {
+            retv = RELOAD_DIALOG;
+        }
     }
     else if ( ( mretv & MENU_ENTRY_DELETE ) && selected_line < rmpd->cmd_list_length ) {
         // Possitive sort index means it is in history.
diff --git a/source/rofi.c b/source/rofi.c
index 314ea097..6eaa5b06 100644
--- a/source/rofi.c
+++ b/source/rofi.c
@@ -208,7 +208,7 @@ static void run_switcher ( ModeMode mode )
 void process_result ( RofiViewState *state )
 {
     Mode *sw = state->sw;
-    rofi_view_set_active ( NULL );
+ //   rofi_view_set_active ( NULL );
     if ( sw != NULL ) {
         unsigned int selected_line = rofi_view_get_selected_line ( state );;
         MenuReturn   mretv         = rofi_view_get_return_value ( state );
@@ -246,11 +246,17 @@ void process_result ( RofiViewState *state )
              * Load in the new mode.
              */
             rofi_view_switch_mode ( state, modi[mode] );
-            rofi_view_set_active ( state );
             curr_switcher = mode;
             return;
+        } else {
+          // On exit, free current view, and pop to one above.
+          rofi_view_remove_active ( state );
+          rofi_view_free ( state );
+          return;
         }
     }
+//    rofi_view_set_active ( NULL );
+          rofi_view_remove_active ( state );
     rofi_view_free ( state );
 }
 
diff --git a/source/view.c b/source/view.c
index 44ce9328..5ba1309a 100644
--- a/source/view.c
+++ b/source/view.c
@@ -415,6 +415,9 @@ static void rofi_view_calculate_window_position ( RofiViewState *state )
 
 static void rofi_view_window_update_size ( RofiViewState * state )
 {
+    if ( state == NULL ) {
+      return;
+    }
     uint16_t mask   = XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y | XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT;
     uint32_t vals[] = { state->x, state->y, state->width, state->height };
 
@@ -490,6 +493,15 @@ RofiViewState * rofi_view_get_active ( void )
     return current_active_menu;
 }
 
+void rofi_view_remove_active ( RofiViewState *state )
+{
+  if ( state == current_active_menu ) {
+    rofi_view_set_active ( NULL );
+  }
+  else if ( state ) {
+    g_queue_remove ( &(CacheState.views ), state);
+  }
+}
 void rofi_view_set_active ( RofiViewState *state )
 {
     if ( current_active_menu != NULL && state != NULL ) {

[DaveDavenport]

Added branch with patch.

[DaveDavenport]

merged in changes. please test.

[dannycolin]

merged in changes. please test.

Works perfectly! One small thing. Maybe we could add a "Press Enter to return to Rofi" in the error message. I built my muscle memory to press ESC, Super + a to quickly reopen a fresh instance of Rofi and I'm guessing I'm not the only one :). The message would help the user discover that new feature.

Also, is there a technical limitation or other reason not to show the message in the same window as Rofi since there's already a widget to show messages? It could prevent disrupting the UX because the user would be able to edit the input content right away.

[DaveDavenport]

Also, is there a technical limitation or other reason not to show the message in the same window as Rofi since there's already a widget to show messages? It could prevent disrupting the UX because the user would be able to edit the input content right away.

This implementation was already there, I just fixed the calling off it. One reason not to use the message widget is that it might already be used/set/reset by the mode.

(escape should also return to the previous view).

[mkhan45]

merged in changes. please test.

It works great for me. Thanks!

[DaveDavenport]

merged in changes. please test.

It works great for me. Thanks!

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.