@techee requested changes on this pull request.
I just added some mostly formal review comments but looks good in general.
> Note that with this change, desktop_startup_id is not freed if Geany exits
> from within parse_command_line_options() -- if this is an issue, I can think
> of refactoring further to ensure this.
Probably not worth it since Geany terminates anyway and this isn't the normal
Geany run which one could e.g. debug with valgrind and see the leak.
> @@ -1055,6 +1055,14 @@ gint main_lib(gint argc, gchar **argv)
#ifdef ENABLE_NLS
main_locale_init(utils_resource_dir(RESOURCE_DIR_LOCALE),
GETTEXT_PACKAGE);
+#endif
+ /* Magic ID used on X11 and Wayland for bringing our existing window on
top;
I'd suggest to add an empty line above the comment for better readability.
> @@ -1055,6 +1055,14 @@ gint main_lib(gint argc, gchar **argv)
#ifdef ENABLE_NLS
main_locale_init(utils_resource_dir(RESOURCE_DIR_LOCALE),
GETTEXT_PACKAGE);
+#endif
+ /* Magic ID used on X11 and Wayland for bringing our existing window on
top;
+ * need to read it here, since GTK will clear this from the environment
in
+ * gtk_init () (called from parse_command_line_options () below). Need
to
+ * make a copy, since the value is not guaranteed to be valid after
calling
+ * unsetenv() (which is done by GTK). */
+#ifdef HAVE_SOCKET
+ gchar *desktop_startup_id = g_strdup(getenv("DESKTOP_STARTUP_ID"));
#endif
Similarly, empty line after `endif`.
> @@ -1085,7 +1093,7 @@ gint main_lib(gint argc, gchar **argv)
#endif
socket_info.lock_socket = -1;
socket_info.lock_socket_tag = 0;
- socket_info.lock_socket = socket_init(argc, argv, socket_port);
+ socket_info.lock_socket = socket_init(argc, argv, socket_port,
desktop_startup_id);
I'd suggest `g_free(desktop_startup_id);` here and dropping the two below.
> {
gint i;
g_return_if_fail(argc > 1);
geany_debug("using running instance of Geany");
+ if (desktop_startup_id != NULL)
+ {
+ geany_debug("sending DESKTOP_STARTUP_ID before file open
command");
This debug message is rather uninteresting because it's from the Geany process
that just sends the ID and quits - so for users there's no way to see it.
I'd suggest adding a similar message to the point where the ID is received and
dropping this one.
> @@ -719,6 +729,22 @@ gboolean socket_lock_input_cb(GIOChannel *source,
> GIOCondition condition, gpoint
cl_options.goto_column = atoi(buf);
}
}
+ else if (strncmp(buf, "desktop_startup_id", 18) == 0)
+ {
+ GdkDisplay *display = gdk_display_get_default();
+ while (socket_fd_gets(sock, buf, sizeof(buf)) != -1 &&
*buf != '.')
+ {
+ g_strstrip(buf); /* remove \n char */
+#if defined(GDK_WINDOWING_WAYLAND) && GTK_CHECK_VERSION(3, 22, 0)
Geany now hard-depends on GTK 3.24 so you can drop the GTK version check here.
> @@ -719,6 +729,22 @@ gboolean socket_lock_input_cb(GIOChannel *source,
> GIOCondition condition, gpoint
cl_options.goto_column = atoi(buf);
}
}
+ else if (strncmp(buf, "desktop_startup_id", 18) == 0)
+ {
+ GdkDisplay *display = gdk_display_get_default();
+ while (socket_fd_gets(sock, buf, sizeof(buf)) != -1 &&
*buf != '.')
Just a note that the 4096 buffer length we use should be more than sufficient.
On Xfce I get tokens like
```
Thunar-1384-debian-gnu-linux-12-geany-7_TIME3166919088
```
> @@ -719,6 +729,22 @@ gboolean socket_lock_input_cb(GIOChannel *source,
> GIOCondition condition, gpoint
cl_options.goto_column = atoi(buf);
}
}
+ else if (strncmp(buf, "desktop_startup_id", 18) == 0)
+ {
+ GdkDisplay *display = gdk_display_get_default();
I think this could cause unused variable warning on Windows or macOS. Maybe
better to move it to both of the `ifdef` blocks.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/4071#pullrequestreview-2461550048
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/4071/review/[email protected]>
