lbonn / rofi

Rofi: A window switcher, run dialog and dmenu replacement - fork with wayland support
Other
927 stars 36 forks source link

We actually don't want PRs to go to next, do we? #79

Closed mcepl closed 1 year ago

mcepl commented 1 year ago

Actually, that’s a serious question: do we?

I have here this patch and I am now considering where to send it: will you ever do sync with the upstream or is this fork final and we should send patches here?

From fa7f73a7a65e967b74e64c6702e01bf399adf037 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C4=9Bj=20Cepl?= <mcepl@cepl.eu>
Date: Sat, 20 Aug 2022 21:51:51 +0200
Subject: [PATCH 1/2] Use standard xdg-terminal instead of the proprietary
 solution.

---
 config/config.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/config/config.c
+++ b/config/config.c
@@ -53,16 +53,16 @@ Settings config = {
     .show_icons = FALSE,

     /** Terminal to use. (for ssh and open in terminal) */
-    .terminal_emulator = "rofi-sensible-terminal",
+    .terminal_emulator = "xdg-terminal",
     .ssh_client = "ssh",
     /** Command when executing ssh. */
-    .ssh_command = "{terminal} -e {ssh-client} {host} [-p {port}]",
+    .ssh_command = "{terminal} {ssh-client} {host} [-p {port}]",
     /** Command when running */
     .run_command = "{cmd}",
     /** Command used to list executable commands. empty -> internal */
     .run_list_command = "",
     /** Command executed when running application in terminal */
-    .run_shell_command = "{terminal} -e {cmd}",
+    .run_shell_command = "{terminal} {cmd}",
     /** Command executed on accep-entry-custom for window modus */
     .window_command = "wmctrl -i -R {window}",
     /** No default icon theme, we search Adwaita and gnome as fallback */
lbonn commented 1 year ago

The changes are fine but could you re-title the commit to something descriptive and not a question?

About your question: I periodically pull from upstream to keep this fork in sync and usually recommend bringing up backend-agnostic changes / features to davatorium/rofi.

Finally about your patch, did you check if xdg-terminal is available in most systems? It looks like it's been part of xdg-utils for a while but I don't see the executable in PATH on a Ubuntu 20.04 I have running:

$ xdg-<TAB>
xdg-dbus-proxy            xdg-email                 xdg-open                  xdg-user-dir
xdg-desktop-icon          xdg-icon-resource         xdg-screensaver           xdg-user-dirs-gtk-update
xdg-desktop-menu          xdg-mime                  xdg-settings              xdg-user-dirs-update
mcepl commented 1 year ago

The situation is apparently even more complicated: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604959 (note how old the bug is). So, I guess, using sane solutions and generally accepted ones is blocked here by Debian. Oh well.