First thoughts:

* I really dislike italics, I think it makes things a lot harder to read, and I 
don't see the need (esp. as the symbol part has a different font).  We had them 
before but only for the non-best match, which is IMO better.  If you wanna keep 
them, let's only do so for those, not for the bold entry.
* I find it looks crowded, making it hard to decipher quickly (thus, harder to 
use): the delimitation between filename and symbol is not visible at a glance.
* I don't think it's wide enough to work properly with C++. When I test it on 
big projects (e.g. LibreOffice), most of the time I just get the return type 
and a tad more, because there are many matches, leading to longer filename, and 
namespaces in the return type making it take some room.  And as mentioned 
previously, as many times it's overrides, it's always the same (the only really 
useful part is the FQN, which is often cut off)

This is not to say there's nothing to like here, but I think it needs improving.
What about that? (not saying it's perfect, I just played a bit with it, but I 
think it's better):
![goto-popup](https://github.com/geany/geany/assets/793526/663dfd78-4aba-4696-aa9b-c9b6e7ea74c2)
![crowded-goto-popup](https://github.com/geany/geany/assets/793526/94087990-18b5-4c16-be38-d802fb297dff)

<details>
<summary>diff</summary>

```diff
diff --git a/src/symbols.c b/src/symbols.c
index 1ee559209..9a25922ec 100644
--- a/src/symbols.c
+++ b/src/symbols.c
@@ -1490,6 +1490,7 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray 
*tags, gboolean have_b
 {
        GtkWidget *first = NULL;
        GtkWidget *menu;
+       GtkSizeGroup *group = gtk_size_group_new(GTK_SIZE_GROUP_HORIZONTAL);
        GdkEvent *event;
        GdkEventButton *button_event = NULL;
        TMTag *tmtag;
@@ -1509,6 +1510,7 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray 
*tags, gboolean have_b
        {
                GtkWidget *item;
                GtkWidget *label;
+               GtkWidget *box;
                GtkWidget *image;
                gchar *fname = short_names[i];
                gchar *text;
@@ -1522,20 +1524,25 @@ static void show_goto_popup(GeanyDocument *doc, 
GPtrArray *tags, gboolean have_b
 
                if (! first && have_best)
                        /* For translators: it's the filename, line number, and 
signature of a symbol in the goto-symbol popup menu */
-                       text = 
g_markup_printf_escaped(_("<i><b>%s:%lu</b></i><small><tt>  %s</tt></small>"), 
fname, tmtag->line, sym);
+                       text = g_markup_printf_escaped(_("<b>%s:%lu</b>"), 
fname, tmtag->line);
                else
                        /* For translators: it's the filename, line number, and 
signature of a symbol in the goto-symbol popup menu */
-                       text = 
g_markup_printf_escaped(_("<i>%s:%lu</i><small><tt>  %s</tt></small>"), fname, 
tmtag->line, sym);
+                       text = g_markup_printf_escaped(_("%s:%lu"), fname, 
tmtag->line);
 
                /* For translators: it's the filename, line number, and 
signature of a symbol in the goto-symbol popup menu */
-               tooltip = 
g_markup_printf_escaped(_("<i>%s:%lu</i>\n<small><tt>%s</tt></small>"), fname, 
tmtag->line, sym);
+               tooltip = 
g_markup_printf_escaped(_("%s:%lu\n<small><tt>%s</tt></small>"), fname, 
tmtag->line, sym);
 
-               g_free(sym);
                image = 
gtk_image_new_from_pixbuf(symbols_icons[get_tag_class(tmtag)].pixbuf);
-               label = g_object_new(GTK_TYPE_LABEL, "label", text, 
"use-markup", TRUE, "xalign", 0.0,
-                                       "tooltip-markup", tooltip, 
"max-width-chars", 80,
-                                       "ellipsize", PANGO_ELLIPSIZE_END, NULL);
-               item = g_object_new(GTK_TYPE_IMAGE_MENU_ITEM, "image", image, 
"child", label, "always-show-image", TRUE, NULL);
+               box = g_object_new(GTK_TYPE_BOX, "orientation", 
GTK_ORIENTATION_HORIZONTAL, "spacing", 12, NULL);
+               label = g_object_new(GTK_TYPE_LABEL, "label", text, 
"use-markup", TRUE, "xalign", 0.0, NULL);
+               gtk_size_group_add_widget(group, label);
+               gtk_box_pack_start(GTK_BOX(box), label, FALSE, FALSE, 0);
+               g_free(text);
+               text = g_markup_printf_escaped("<small><tt>%s</tt></small>", 
sym);
+               gtk_box_pack_start(GTK_BOX(box), g_object_new(GTK_TYPE_LABEL, 
"label", text, "use-markup", TRUE, "xalign", 0.0,
+                                       "max-width-chars", 80, "ellipsize", 
PANGO_ELLIPSIZE_END, NULL), FALSE, FALSE, 0);
+               item = g_object_new(GTK_TYPE_IMAGE_MENU_ITEM, "image", image, 
"child", box, "always-show-image", TRUE,
+                                       "tooltip-markup", tooltip, NULL);
                g_signal_connect_data(item, "activate", 
G_CALLBACK(on_goto_popup_item_activate),
                                      tm_tag_ref(tmtag), (GClosureNotify) 
tm_tag_unref, 0);
                gtk_menu_shell_append(GTK_MENU_SHELL(menu), item);
@@ -1543,6 +1550,7 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray 
*tags, gboolean have_b
                if (! first)
                        first = item;
 
+               g_free(sym);
                g_free(text);
                g_free(fname);
        }
@@ -1563,6 +1571,8 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray 
*tags, gboolean have_b
                               button_event ? (GDestroyNotify) gdk_event_free : 
NULL);
        ui_menu_popup(GTK_MENU(menu), goto_popup_position_func, 
doc->editor->sci,
                                  button_event ? button_event->button : 0, 
gtk_get_current_event_time ());
+
+       g_object_unref(group);
 }
 
 
```
</details>

-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3547#issuecomment-1751802744
You are receiving this because you are subscribed to this thread.

Message ID: <geany/geany/pull/3547/c1751802...@github.com>

Reply via email to