From eb2d1d8fc894ae7ebba24019f670e32fc48c031e Mon Sep 17 00:00:00 2001 From: Max Gautier Date: Sat, 23 Jul 2022 20:16:11 +0200 Subject: [PATCH 1/4] gio: Refactor the known terminals search Get rid of multiple conditionals branch by using a loop and storing the options needed by particular terminal emulators directly in an array. Remove intermediate variable term_argv as we don't need it. Advantages: - simpler logic, less branching - the terminal emulator list is more readable, by virtue of being condensed in one array. Launch options to execute a terminal program are also more explicitly specified - the logic become independent from the order - one less allocation --- gio/gdesktopappinfo.c | 95 ++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 59 deletions(-) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index f8cccca02..94e1f7fd9 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -2627,12 +2627,27 @@ prepend_terminal_to_vector (int *argc, { #ifndef G_OS_WIN32 char **real_argv; - int real_argc; - int i, j; - char **term_argv = NULL; - int term_argc = 0; - char *check; + size_t real_argc; + size_t i; + int term_argc = 2; + char *found_terminal; char **the_argv; + const char *term_arg; + static const struct { + const char *exec; + const char *exec_arg; + } known_terminals[] = { + { "gnome-terminal", "--" }, + { "mate-terminal", "-x" }, + { "xfce4-terminal", "-x" }, + { "tilix", "-e" }, + { "konsole", "-e" }, + { "nxterm", "-e" }, + { "color-xterm", "-e" }, + { "rxvt", "-e" }, + { "dtterm", "-e" }, + { "xterm", "-e" } + }; g_return_val_if_fail (argc != NULL, FALSE); g_return_val_if_fail (argv != NULL, FALSE); @@ -2646,69 +2661,34 @@ prepend_terminal_to_vector (int *argc, /* compute size if not given */ if (*argc < 0) { - for (i = 0; the_argv[i] != NULL; i++) + for ((*argc) = 0; the_argv[*argc] != NULL; (*argc)++) ; - *argc = i; } - term_argc = 2; - term_argv = g_new0 (char *, 3); - - check = g_find_program_in_path ("gnome-terminal"); - if (check != NULL) + for (i = 0, found_terminal = NULL; i < G_N_ELEMENTS (known_terminals); i++) { - term_argv[0] = check; - /* Since 2017, gnome-terminal has preferred `--` over `-x` or `-e`. */ - term_argv[1] = g_strdup ("--"); + found_terminal = g_find_program_in_path (known_terminals[i].exec); + if (found_terminal != NULL) + { + term_arg = known_terminals[i].exec_arg; + break; + } } - else + + if (found_terminal == NULL) { - if (check == NULL) - check = g_find_program_in_path ("mate-terminal"); - if (check == NULL) - check = g_find_program_in_path ("xfce4-terminal"); - if (check != NULL) - { - term_argv[0] = check; - /* Note that gnome-terminal takes -x and - * as -e in gnome-terminal is broken we use that. */ - term_argv[1] = g_strdup ("-x"); - } - else - { - if (check == NULL) - check = g_find_program_in_path ("tilix"); - if (check == NULL) - check = g_find_program_in_path ("konsole"); - if (check == NULL) - check = g_find_program_in_path ("nxterm"); - if (check == NULL) - check = g_find_program_in_path ("color-xterm"); - if (check == NULL) - check = g_find_program_in_path ("rxvt"); - if (check == NULL) - check = g_find_program_in_path ("dtterm"); - if (check == NULL) - check = g_find_program_in_path ("xterm"); - if (check == NULL) - { - g_debug ("Couldn’t find a known terminal"); - g_free (term_argv); - return FALSE; - } - term_argv[0] = check; - term_argv[1] = g_strdup ("-e"); - } + g_debug ("Couldn’t find a known terminal"); + return FALSE; } real_argc = term_argc + *argc; real_argv = g_new (char *, real_argc + 1); - for (i = 0; i < term_argc; i++) - real_argv[i] = term_argv[i]; + real_argv[0] = found_terminal; + real_argv[1] = g_strdup (term_arg); - for (j = 0; j < *argc; j++, i++) - real_argv[i] = (char *)the_argv[j]; + for (i = term_argc; i < real_argc; i++) + real_argv[i] = the_argv[i - term_argc]; real_argv[i] = NULL; @@ -2716,9 +2696,6 @@ prepend_terminal_to_vector (int *argc, *argv = real_argv; *argc = real_argc; - /* we use g_free here as we sucked all the inner strings - * out from it into real_argv */ - g_free (term_argv); return TRUE; #else return FALSE; From b64347d279e05cd2b1f960c22ec73f8529443ded Mon Sep 17 00:00:00 2001 From: Max Gautier Date: Sat, 23 Jul 2022 22:00:53 +0200 Subject: [PATCH 2/4] gio: add support for terminal with no option Introduce support for terminals executing commands without an option, i.e., the command is passed directly as argument to the terminal emulator. This is needed for xdg-terminal-exec. --- gio/gdesktopappinfo.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index 94e1f7fd9..eb833b110 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -2629,7 +2629,7 @@ prepend_terminal_to_vector (int *argc, char **real_argv; size_t real_argc; size_t i; - int term_argc = 2; + size_t term_argc; char *found_terminal; char **the_argv; const char *term_arg; @@ -2681,14 +2681,21 @@ prepend_terminal_to_vector (int *argc, return FALSE; } + /* check if the terminal require an option */ + term_argc = term_arg ? 2 : 1; + real_argc = term_argc + *argc; real_argv = g_new (char *, real_argc + 1); - real_argv[0] = found_terminal; - real_argv[1] = g_strdup (term_arg); + i = 0; + real_argv[i++] = found_terminal; - for (i = term_argc; i < real_argc; i++) - real_argv[i] = the_argv[i - term_argc]; + if (term_arg) + real_argv[i++] = g_strdup (term_arg); + + g_assert (i == term_argc); + for (int j = 0; j < *argc; j++) + real_argv[i++] = the_argv[j]; real_argv[i] = NULL; From 22e1b9bcc0ca7cd1ba2457ddf5b5545752f9c7ea Mon Sep 17 00:00:00 2001 From: Max Gautier Date: Sat, 23 Jul 2022 22:00:53 +0200 Subject: [PATCH 3/4] gio: add xdg-terminal-exec as a known terminal Allow users to select their terminal of choice by using the xdg-terminal-exec wrapper. It is a temporary temporary solution while waiting for the xdg-default-apps specification (https://gitlab.freedesktop.org/xdg/xdg-specs/-/issues/54), in accordance with this comment: https://gitlab.gnome.org/GNOME/glib/-/issues/338#note_1076172 --- gio/gdesktopappinfo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index eb833b110..6289c8c6f 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -2637,6 +2637,7 @@ prepend_terminal_to_vector (int *argc, const char *exec; const char *exec_arg; } known_terminals[] = { + { "xdg-terminal-exec", NULL }, { "gnome-terminal", "--" }, { "mate-terminal", "-x" }, { "xfce4-terminal", "-x" }, From dd9bc7cf5956740f94acb4a0a7cd4945a9701449 Mon Sep 17 00:00:00 2001 From: Max Gautier Date: Wed, 26 Oct 2022 11:05:13 +0200 Subject: [PATCH 4/4] gio: test xdg-terminal-exec usage --- gio/tests/desktop-app-info.c | 41 +++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/gio/tests/desktop-app-info.c b/gio/tests/desktop-app-info.c index 5248ca9ed..39e2e4f29 100644 --- a/gio/tests/desktop-app-info.c +++ b/gio/tests/desktop-app-info.c @@ -1187,6 +1187,8 @@ test_id (void) static const char * get_terminal_divider (const char *terminal_name) { + if (g_str_equal (terminal_name, "xdg-terminal-exec")) + return NULL; if (g_str_equal (terminal_name, "gnome-terminal")) return "--"; if (g_str_equal (terminal_name, "tilix")) @@ -1217,6 +1219,7 @@ test_launch_uris_with_terminal (gconstpointer data) int fd; int ret; int flags; + int terminal_divider_arg_length; const char *terminal_exec = data; char *old_path; char *command_line; @@ -1312,12 +1315,21 @@ test_launch_uris_with_terminal (gconstpointer data) output_args = g_strsplit (output_contents, " ", -1); g_clear_pointer (&output_contents, g_free); - g_assert_cmpuint (g_strv_length (output_args), ==, 4); - g_assert_cmpstr (output_args[0], ==, get_terminal_divider (terminal_exec)); - g_assert_cmpstr (output_args[1], ==, "true"); - g_assert_cmpstr (output_args[2], ==, command_line + 5); + terminal_divider_arg_length = (get_terminal_divider (terminal_exec) != NULL) ? 1 : 0; + g_assert_cmpuint (g_strv_length (output_args), ==, 3 + terminal_divider_arg_length); + if (terminal_divider_arg_length == 1) + { + g_assert_cmpstr (output_args[0], ==, get_terminal_divider (terminal_exec)); + g_assert_cmpstr (output_args[1], ==, "true"); + g_assert_cmpstr (output_args[2], ==, command_line + 5); + } + else + { + g_assert_cmpstr (output_args[0], ==, "true"); + g_assert_cmpstr (output_args[1], ==, command_line + 5); + } paths = g_list_delete_link (paths, - g_list_find_custom (paths, output_args[3], g_str_equal)); + g_list_find_custom (paths, output_args[2 + terminal_divider_arg_length], g_str_equal)); g_assert_cmpint (g_list_length (paths), ==, 1); g_clear_pointer (&output_args, g_strfreev); @@ -1337,12 +1349,20 @@ test_launch_uris_with_terminal (gconstpointer data) output_args = g_strsplit (output_contents, " ", -1); g_clear_pointer (&output_contents, g_free); - g_assert_cmpuint (g_strv_length (output_args), ==, 4); - g_assert_cmpstr (output_args[0], ==, get_terminal_divider (terminal_exec)); - g_assert_cmpstr (output_args[1], ==, "true"); - g_assert_cmpstr (output_args[2], ==, command_line + 5); + g_assert_cmpuint (g_strv_length (output_args), ==, 3 + terminal_divider_arg_length); + if (terminal_divider_arg_length > 0) + { + g_assert_cmpstr (output_args[0], ==, get_terminal_divider (terminal_exec)); + g_assert_cmpstr (output_args[1], ==, "true"); + g_assert_cmpstr (output_args[2], ==, command_line + 5); + } + else + { + g_assert_cmpstr (output_args[0], ==, "true"); + g_assert_cmpstr (output_args[1], ==, command_line + 5); + } paths = g_list_delete_link (paths, - g_list_find_custom (paths, output_args[3], g_str_equal)); + g_list_find_custom (paths, output_args[2 + terminal_divider_arg_length], g_str_equal)); g_assert_cmpint (g_list_length (paths), ==, 0); g_clear_pointer (&output_args, g_strfreev); @@ -1408,6 +1428,7 @@ main (int argc, { guint i; const gchar *supported_terminals[] = { + "xdg-terminal-exec", "gnome-terminal", "mate-terminal", "xfce4-terminal",