From 4c0c76e6f40e0ad12fa1aac676872200b948a45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Techet?= Date: Tue, 14 Jul 2015 13:31:50 +0200 Subject: [PATCH 01/10] If more tags are found during tag definition/declaration goto, let user select which one to use If only a single tag is found, just perform the goto. If more tags are found, show them in a popup. Try to help the user find the right file by putting the "best" tag at the first position and emphasizing it. Thanks to Colomban Wendling for various improvements of this patch. --- src/symbols.c | 367 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 293 insertions(+), 74 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index db375145..79b72d53 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -54,6 +54,7 @@ #include "tm_tag.h" #include "ui_utils.h" #include "utils.h" +#include "keybindings.h" #include "SciLexer.h" @@ -62,6 +63,7 @@ #include #include #include +#include static gchar **html_entities = NULL; @@ -84,6 +86,13 @@ enum /* Geany tag files */ GTF_MAX }; +enum { + PIXBUF_COLUMN, + TEXT_COLUMN, + TAG_COLUMN, + N_COLUMNS +}; + static TagFileInfo tag_file_info[GTF_MAX] = { {FALSE, "c99.tags"}, @@ -138,6 +147,9 @@ static struct } symbol_menu; +static GtkWidget *tag_goto_popup = NULL; +static GtkWidget *tag_goto_tree_view = NULL; + static void html_tags_loaded(void); static void load_user_tags(filetype_id ft_id); @@ -326,66 +338,6 @@ const gchar *symbols_get_context_separator(gint ft_id) } -/* Note: if tags is sorted, we can use bsearch or tm_tags_find() to speed this up. */ -static TMTag * -symbols_find_tm_tag(const GPtrArray *tags, const gchar *tag_name) -{ - guint i; - g_return_val_if_fail(tags != NULL, NULL); - - for (i = 0; i < tags->len; ++i) - { - if (utils_str_equal(TM_TAG(tags->pdata[i])->name, tag_name)) - return TM_TAG(tags->pdata[i]); - } - return NULL; -} - - -static TMTag *find_source_file_tag(GPtrArray *tags_array, - const gchar *tag_name, guint type) -{ - GPtrArray *tags; - TMTag *tmtag; - - tags = tm_tags_extract(tags_array, type); - if (tags != NULL) - { - tmtag = symbols_find_tm_tag(tags, tag_name); - - g_ptr_array_free(tags, TRUE); - - if (tmtag != NULL) - return tmtag; - } - return NULL; /* not found */ -} - - -static TMTag *find_workspace_tag(const gchar *tag_name, guint type) -{ - guint j; - const GPtrArray *source_files = NULL; - - if (app->tm_workspace != NULL) - source_files = app->tm_workspace->source_files; - - if (source_files != NULL) - { - for (j = 0; j < source_files->len; j++) - { - TMSourceFile *srcfile = source_files->pdata[j]; - TMTag *tmtag; - - tmtag = find_source_file_tag(srcfile->tags_array, tag_name, type); - if (tmtag != NULL) - return tmtag; - } - } - return NULL; /* not found */ -} - - const gchar **symbols_get_html_entities(void) { if (html_entities == NULL) @@ -1888,27 +1840,270 @@ static void load_user_tags(filetype_id ft_id) } +static void on_focus_out(GtkWidget *list, GdkEventFocus *unused, gpointer *user_data) +{ + gtk_widget_hide(tag_goto_popup); +} + + +static void on_row_activated(GtkTreeView *tree_view, GtkTreePath *path, + GtkTreeViewColumn *column, gpointer user_data) +{ + GtkTreeModel *model = gtk_tree_view_get_model(tree_view); + GeanyDocument *new_doc, *old_doc; + GtkTreeIter iter; + TMTag *tag; + + gtk_tree_model_get_iter(model, &iter, path); + gtk_tree_model_get(model, &iter, TAG_COLUMN, &tag, -1); + g_return_if_fail(tag); + + old_doc = document_get_current(); + new_doc = document_open_file(tag->file->file_name, FALSE, NULL, NULL); + + if (new_doc) + navqueue_goto_line(old_doc, new_doc, tag->line); + + gtk_widget_hide(tag_goto_popup); + tm_tag_unref(tag); +} + + +static gboolean on_key_pressed(GtkWidget *widget, GdkEventKey *event, gpointer user_data) +{ + guint state = event->state & gtk_accelerator_get_default_mod_mask(); + + if (event->keyval == GDK_Escape && state == 0) + { + gtk_widget_hide(tag_goto_popup); + keybindings_send_command(GEANY_KEY_GROUP_FOCUS, GEANY_KEYS_FOCUS_EDITOR); + return TRUE; + } + return FALSE; +} + + +/* FIXME: use the same icons as in the symbols tree defined in add_top_level_items() */ +static guint get_tag_class(const TMTag *tag) +{ + switch (tag->type) + { + case tm_tag_prototype_t: + case tm_tag_method_t: + case tm_tag_function_t: + return ICON_METHOD; + case tm_tag_variable_t: + case tm_tag_externvar_t: + return ICON_VAR; + case tm_tag_macro_t: + case tm_tag_macro_with_arg_t: + return ICON_MACRO; + case tm_tag_class_t: + return ICON_CLASS; + case tm_tag_member_t: + case tm_tag_field_t: + return ICON_MEMBER; + case tm_tag_typedef_t: + case tm_tag_enum_t: + case tm_tag_union_t: + case tm_tag_struct_t: + return ICON_STRUCT; + case tm_tag_namespace_t: + case tm_tag_package_t: + return ICON_NAMESPACE; + default: + break; + } + return ICON_STRUCT; +} + + +static void create_goto_popup(void) +{ + GtkWidget *frame, *scroller; + GtkListStore *store; + GtkTreeSelection *selection; + GtkTreeViewColumn *column; + GtkCellRenderer *renderer; + + tag_goto_popup = g_object_new(GTK_TYPE_WINDOW, "type", GTK_WINDOW_TOPLEVEL, NULL); + gtk_widget_set_can_focus(tag_goto_popup, TRUE); + gtk_window_set_type_hint(GTK_WINDOW(tag_goto_popup), GDK_WINDOW_TYPE_HINT_DIALOG); + gtk_window_set_decorated(GTK_WINDOW(tag_goto_popup), FALSE); + gtk_window_set_transient_for(GTK_WINDOW(tag_goto_popup), GTK_WINDOW(main_widgets.window)); + gtk_window_set_destroy_with_parent(GTK_WINDOW(tag_goto_popup), TRUE); + gtk_window_set_position(GTK_WINDOW(tag_goto_popup), GTK_WIN_POS_CENTER_ON_PARENT); + gtk_widget_set_size_request(tag_goto_popup, 250, 150); + + frame = gtk_frame_new(NULL); + gtk_container_add(GTK_CONTAINER(tag_goto_popup), frame); + gtk_frame_set_shadow_type(GTK_FRAME(frame), GTK_SHADOW_OUT); + gtk_container_set_border_width(GTK_CONTAINER(frame), 0); + + scroller = gtk_scrolled_window_new(NULL, NULL); + gtk_container_set_border_width(GTK_CONTAINER(scroller), 0); + gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scroller), + GTK_POLICY_NEVER, GTK_POLICY_AUTOMATIC); + gtk_container_add(GTK_CONTAINER(frame), scroller); + + /* TreeView and its model */ + store = gtk_list_store_new(N_COLUMNS, GDK_TYPE_PIXBUF, G_TYPE_STRING, TM_TYPE_TAG); + tag_goto_tree_view = gtk_tree_view_new_with_model(GTK_TREE_MODEL(store)); + gtk_widget_set_can_focus(tag_goto_tree_view, TRUE); + gtk_container_add(GTK_CONTAINER(scroller), tag_goto_tree_view); + g_signal_connect(tag_goto_tree_view, "focus-out-event", G_CALLBACK(on_focus_out), tag_goto_popup); + g_signal_connect(tag_goto_tree_view, "row-activated", G_CALLBACK(on_row_activated), NULL); + g_signal_connect(tag_goto_tree_view, "key-press-event", G_CALLBACK(on_key_pressed), NULL); + + selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(tag_goto_tree_view)); + gtk_tree_selection_set_mode(selection, GTK_SELECTION_SINGLE); + gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(tag_goto_tree_view), FALSE); + gtk_tree_view_set_reorderable(GTK_TREE_VIEW(tag_goto_tree_view), FALSE); + + /* Columns */ + column = gtk_tree_view_column_new(); + gtk_tree_view_column_set_sizing(column, GTK_TREE_VIEW_COLUMN_FIXED); + + renderer = gtk_cell_renderer_pixbuf_new(); + gtk_tree_view_column_pack_start(column, renderer, FALSE); + gtk_tree_view_column_add_attribute(column, renderer, "pixbuf", PIXBUF_COLUMN); + + renderer = gtk_cell_renderer_text_new(); + gtk_tree_view_column_pack_start(column, renderer, TRUE); + gtk_tree_view_column_add_attribute(column, renderer, "markup", TEXT_COLUMN); + + gtk_tree_view_append_column(GTK_TREE_VIEW(tag_goto_tree_view), column); + + gtk_widget_show_all(frame); +} + + +static void show_goto_popup(GPtrArray *tags, gboolean have_best) +{ + gboolean first = TRUE; + GtkTreePath *path; + GtkTreeModel *model; + GtkTreeIter iter; + TMTag *tmtag; + guint i; + + if (!tag_goto_popup) + create_goto_popup(); + + model = gtk_tree_view_get_model(GTK_TREE_VIEW(tag_goto_tree_view)); + gtk_list_store_clear(GTK_LIST_STORE(model)); + + foreach_ptr_array(tmtag, i, tags) + { + gchar *fname = g_path_get_basename(tmtag->file->file_name); + gchar *text; + + if (first && have_best) + text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); + else + text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); + + gtk_list_store_insert_with_values(GTK_LIST_STORE(model), &iter, -1, + PIXBUF_COLUMN, symbols_icons[get_tag_class(tmtag)].pixbuf, + TEXT_COLUMN, text, + TAG_COLUMN, tmtag, -1); + + g_free(text); + g_free(fname); + first = FALSE; + } + + path = gtk_tree_path_new_first(); + gtk_tree_view_set_cursor (GTK_TREE_VIEW(tag_goto_tree_view), path, NULL, FALSE); + gtk_tree_path_free(path); + + gtk_window_present(GTK_WINDOW(tag_goto_popup)); +} + + +static gint compare_tags_by_name_line(gconstpointer ptr1, gconstpointer ptr2) +{ + gint res; + TMTag *t1 = *((TMTag **) ptr1); + TMTag *t2 = *((TMTag **) ptr2); + + res = g_strcmp0(t1->file->short_name, t2->file->short_name); + if (res != 0) + return res; + return t1->line - t2->line; +} + + +static TMTag *find_best_goto_tag(GeanyDocument *doc, GPtrArray *tags) +{ + TMTag *tag; + guint i; + + /* first check if we have a tag in the current file */ + foreach_ptr_array(tag, i, tags) + { + if (g_strcmp0(doc->real_path, tag->file->file_name) == 0) + return tag; + } + + /* next check if we have a tag for some of the open documents */ + foreach_ptr_array(tag, i, tags) + { + guint j; + + foreach_document(j) + { + if (g_strcmp0(documents[j]->real_path, tag->file->file_name) == 0) + return tag; + } + } + + /* next check if we have a tag for a file inside the current document's directory */ + foreach_ptr_array(tag, i, tags) + { + gchar *dir = g_path_get_dirname(doc->real_path); + + if (g_str_has_prefix(tag->file->file_name, dir)) + { + g_free(dir); + return tag; + } + g_free(dir); + } + + return NULL; +} + + static gboolean goto_tag(const gchar *name, gboolean definition) { const TMTagType forward_types = tm_tag_prototype_t | tm_tag_externvar_t; TMTagType type; TMTag *tmtag = NULL; GeanyDocument *old_doc = document_get_current(); + gboolean found = FALSE; + const GPtrArray *all_tags; + GPtrArray *workspace_tags; + guint i; /* goto tag definition: all except prototypes / forward declarations / externs */ type = (definition) ? tm_tag_max_t - forward_types : forward_types; + all_tags = tm_workspace_find(name, NULL, type, NULL, old_doc->file_type->lang); - /* first look in the current document */ - if (old_doc != NULL && old_doc->tm_file) - tmtag = find_source_file_tag(old_doc->tm_file->tags_array, name, type); - - /* if not found, look in the workspace */ - if (tmtag == NULL) - tmtag = find_workspace_tag(name, type); - - if (tmtag != NULL) + /* get rid of global tags */ + workspace_tags = g_ptr_array_new(); + foreach_ptr_array(tmtag, i, all_tags) { - GeanyDocument *new_doc = document_find_by_real_path( + if (tmtag->file) + g_ptr_array_add(workspace_tags, tmtag); + } + + if (workspace_tags->len == 1) + { + GeanyDocument *new_doc; + + tmtag = workspace_tags->pdata[0]; + new_doc = document_find_by_real_path( tmtag->file->file_name); if (new_doc) @@ -1918,7 +2113,7 @@ static gboolean goto_tag(const gchar *name, gboolean definition) tmtag->line == (guint)sci_get_current_line(old_doc->editor->sci) + 1) { if (goto_tag(name, !definition)) - return TRUE; + found = TRUE; } } else @@ -1927,10 +2122,34 @@ static gboolean goto_tag(const gchar *name, gboolean definition) new_doc = document_open_file(tmtag->file->file_name, FALSE, NULL, NULL); } - if (navqueue_goto_line(old_doc, new_doc, tmtag->line)) - return TRUE; + if (!found && navqueue_goto_line(old_doc, new_doc, tmtag->line)) + found = TRUE; } - return FALSE; + else if (workspace_tags->len > 1) + { + GPtrArray *tags; + TMTag *tag, *best_tag; + + g_ptr_array_sort(workspace_tags, compare_tags_by_name_line); + best_tag = find_best_goto_tag(old_doc, workspace_tags); + + tags = g_ptr_array_new(); + if (best_tag) + g_ptr_array_add(tags, best_tag); + foreach_ptr_array(tag, i, workspace_tags) + { + if (tag != best_tag) + g_ptr_array_add(tags, tag); + } + show_goto_popup(tags, best_tag != NULL); + + g_ptr_array_free(tags, TRUE); + found = TRUE; + } + + g_ptr_array_free(workspace_tags, TRUE); + + return found; } From 943bfa52c5f98e3adc0f8581845e492203c1218c Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Mon, 15 Feb 2016 17:45:25 +0100 Subject: [PATCH 02/10] Re-implement goto tag popup with a poped-up GtkMenu --- src/symbols.c | 171 ++++++++++++++++---------------------------------- 1 file changed, 54 insertions(+), 117 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index 79b72d53..113b1fe0 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -54,7 +54,6 @@ #include "tm_tag.h" #include "ui_utils.h" #include "utils.h" -#include "keybindings.h" #include "SciLexer.h" @@ -63,7 +62,6 @@ #include #include #include -#include static gchar **html_entities = NULL; @@ -86,13 +84,6 @@ enum /* Geany tag files */ GTF_MAX }; -enum { - PIXBUF_COLUMN, - TEXT_COLUMN, - TAG_COLUMN, - N_COLUMNS -}; - static TagFileInfo tag_file_info[GTF_MAX] = { {FALSE, "c99.tags"}, @@ -147,9 +138,6 @@ static struct } symbol_menu; -static GtkWidget *tag_goto_popup = NULL; -static GtkWidget *tag_goto_tree_view = NULL; - static void html_tags_loaded(void); static void load_user_tags(filetype_id ft_id); @@ -1840,22 +1828,10 @@ static void load_user_tags(filetype_id ft_id) } -static void on_focus_out(GtkWidget *list, GdkEventFocus *unused, gpointer *user_data) +static void on_goto_popup_item_activate(GtkMenuItem *item, TMTag *tag) { - gtk_widget_hide(tag_goto_popup); -} - - -static void on_row_activated(GtkTreeView *tree_view, GtkTreePath *path, - GtkTreeViewColumn *column, gpointer user_data) -{ - GtkTreeModel *model = gtk_tree_view_get_model(tree_view); GeanyDocument *new_doc, *old_doc; - GtkTreeIter iter; - TMTag *tag; - gtk_tree_model_get_iter(model, &iter, path); - gtk_tree_model_get(model, &iter, TAG_COLUMN, &tag, -1); g_return_if_fail(tag); old_doc = document_get_current(); @@ -1863,23 +1839,6 @@ static void on_row_activated(GtkTreeView *tree_view, GtkTreePath *path, if (new_doc) navqueue_goto_line(old_doc, new_doc, tag->line); - - gtk_widget_hide(tag_goto_popup); - tm_tag_unref(tag); -} - - -static gboolean on_key_pressed(GtkWidget *widget, GdkEventKey *event, gpointer user_data) -{ - guint state = event->state & gtk_accelerator_get_default_mod_mask(); - - if (event->keyval == GDK_Escape && state == 0) - { - gtk_widget_hide(tag_goto_popup); - keybindings_send_command(GEANY_KEY_GROUP_FOCUS, GEANY_KEYS_FOCUS_EDITOR); - return TRUE; - } - return FALSE; } @@ -1918,106 +1877,84 @@ static guint get_tag_class(const TMTag *tag) } -static void create_goto_popup(void) +/* positions a popup below the caret from the ScintillaObject in @p data */ +static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer data) { - GtkWidget *frame, *scroller; - GtkListStore *store; - GtkTreeSelection *selection; - GtkTreeViewColumn *column; - GtkCellRenderer *renderer; + ScintillaObject *sci = data; + GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(sci)); + gint pos = sci_get_current_position(sci); + gint line = sci_get_line_from_position(sci, pos); + gint line_height = scintilla_send_message(sci, SCI_TEXTHEIGHT, line, 0); + gint pos_x = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos); + gint pos_y = scintilla_send_message(sci, SCI_POINTYFROMPOSITION, 0, pos); - tag_goto_popup = g_object_new(GTK_TYPE_WINDOW, "type", GTK_WINDOW_TOPLEVEL, NULL); - gtk_widget_set_can_focus(tag_goto_popup, TRUE); - gtk_window_set_type_hint(GTK_WINDOW(tag_goto_popup), GDK_WINDOW_TYPE_HINT_DIALOG); - gtk_window_set_decorated(GTK_WINDOW(tag_goto_popup), FALSE); - gtk_window_set_transient_for(GTK_WINDOW(tag_goto_popup), GTK_WINDOW(main_widgets.window)); - gtk_window_set_destroy_with_parent(GTK_WINDOW(tag_goto_popup), TRUE); - gtk_window_set_position(GTK_WINDOW(tag_goto_popup), GTK_WIN_POS_CENTER_ON_PARENT); - gtk_widget_set_size_request(tag_goto_popup, 250, 150); + gdk_window_get_origin(window, x, y); - frame = gtk_frame_new(NULL); - gtk_container_add(GTK_CONTAINER(tag_goto_popup), frame); - gtk_frame_set_shadow_type(GTK_FRAME(frame), GTK_SHADOW_OUT); - gtk_container_set_border_width(GTK_CONTAINER(frame), 0); - - scroller = gtk_scrolled_window_new(NULL, NULL); - gtk_container_set_border_width(GTK_CONTAINER(scroller), 0); - gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scroller), - GTK_POLICY_NEVER, GTK_POLICY_AUTOMATIC); - gtk_container_add(GTK_CONTAINER(frame), scroller); - - /* TreeView and its model */ - store = gtk_list_store_new(N_COLUMNS, GDK_TYPE_PIXBUF, G_TYPE_STRING, TM_TYPE_TAG); - tag_goto_tree_view = gtk_tree_view_new_with_model(GTK_TREE_MODEL(store)); - gtk_widget_set_can_focus(tag_goto_tree_view, TRUE); - gtk_container_add(GTK_CONTAINER(scroller), tag_goto_tree_view); - g_signal_connect(tag_goto_tree_view, "focus-out-event", G_CALLBACK(on_focus_out), tag_goto_popup); - g_signal_connect(tag_goto_tree_view, "row-activated", G_CALLBACK(on_row_activated), NULL); - g_signal_connect(tag_goto_tree_view, "key-press-event", G_CALLBACK(on_key_pressed), NULL); - - selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(tag_goto_tree_view)); - gtk_tree_selection_set_mode(selection, GTK_SELECTION_SINGLE); - gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(tag_goto_tree_view), FALSE); - gtk_tree_view_set_reorderable(GTK_TREE_VIEW(tag_goto_tree_view), FALSE); - - /* Columns */ - column = gtk_tree_view_column_new(); - gtk_tree_view_column_set_sizing(column, GTK_TREE_VIEW_COLUMN_FIXED); - - renderer = gtk_cell_renderer_pixbuf_new(); - gtk_tree_view_column_pack_start(column, renderer, FALSE); - gtk_tree_view_column_add_attribute(column, renderer, "pixbuf", PIXBUF_COLUMN); - - renderer = gtk_cell_renderer_text_new(); - gtk_tree_view_column_pack_start(column, renderer, TRUE); - gtk_tree_view_column_add_attribute(column, renderer, "markup", TEXT_COLUMN); - - gtk_tree_view_append_column(GTK_TREE_VIEW(tag_goto_tree_view), column); - - gtk_widget_show_all(frame); + *x += pos_x; + *y += pos_y + line_height; + *push_in = TRUE; } -static void show_goto_popup(GPtrArray *tags, gboolean have_best) +static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_best) { - gboolean first = TRUE; - GtkTreePath *path; - GtkTreeModel *model; - GtkTreeIter iter; + GtkWidget *first = NULL; + GtkWidget *menu; + GdkEvent *event; TMTag *tmtag; guint i; - if (!tag_goto_popup) - create_goto_popup(); - - model = gtk_tree_view_get_model(GTK_TREE_VIEW(tag_goto_tree_view)); - gtk_list_store_clear(GTK_LIST_STORE(model)); + menu = gtk_menu_new(); foreach_ptr_array(tmtag, i, tags) { + GtkWidget *item; + GtkWidget *label; + GtkWidget *image; gchar *fname = g_path_get_basename(tmtag->file->file_name); gchar *text; - if (first && have_best) + if (! first && have_best) text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); else text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); - gtk_list_store_insert_with_values(GTK_LIST_STORE(model), &iter, -1, - PIXBUF_COLUMN, symbols_icons[get_tag_class(tmtag)].pixbuf, - TEXT_COLUMN, text, - TAG_COLUMN, tmtag, -1); + 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, NULL); + item = g_object_new(GTK_TYPE_IMAGE_MENU_ITEM, "image", image, "child", label, 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); + + if (! first) + first = item; g_free(text); g_free(fname); - first = FALSE; } - path = gtk_tree_path_new_first(); - gtk_tree_view_set_cursor (GTK_TREE_VIEW(tag_goto_tree_view), path, NULL, FALSE); - gtk_tree_path_free(path); + gtk_widget_show_all(menu); - gtk_window_present(GTK_WINDOW(tag_goto_popup)); + /* FIXME: this should get the real event directly instead of looking it up */ + event = gtk_get_current_event(); + if (event && event->type == GDK_BUTTON_PRESS) + { + GdkEventButton *event_button = (GdkEventButton *) event; + /* FIXME: should this also use the position func? as the cursor must be on the location + * under the cursor at this point anyway, it might give prettier alignment. + * But might as well be farther from the pointer or otherwise misaligned with the + * pointer, so maybe not. */ + gtk_menu_popup(GTK_MENU(menu), NULL, NULL, NULL, NULL, event_button->button, event_button->time); + } + else + { + if (first) /* always select the first item for better keyboard navigation */ + g_signal_connect(menu, "realize", G_CALLBACK(gtk_menu_shell_select_item), first); + gtk_menu_popup(GTK_MENU(menu), NULL, NULL, goto_popup_position_func, doc->editor->sci, + 0, gtk_get_current_event_time ()); + } + if (event) + gdk_event_free(event); } @@ -2141,7 +2078,7 @@ static gboolean goto_tag(const gchar *name, gboolean definition) if (tag != best_tag) g_ptr_array_add(tags, tag); } - show_goto_popup(tags, best_tag != NULL); + show_goto_popup(old_doc, tags, best_tag != NULL); g_ptr_array_free(tags, TRUE); found = TRUE; From 61582a42f9a7acb86a2b2b79b17fb5fc80e86755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Techet?= Date: Thu, 18 Feb 2016 22:11:08 +0100 Subject: [PATCH 03/10] Always select the first item for better keyboard manipulation Even when the user Ctrl+clicks to perform goto tag definition, it should be possible to select the item from the list using keyboard (and have the first item automatically selected so ctrl+click plus enter afterwards always gets you somewhere). --- src/symbols.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index 113b1fe0..34663d49 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1935,26 +1935,10 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_b gtk_widget_show_all(menu); - /* FIXME: this should get the real event directly instead of looking it up */ - event = gtk_get_current_event(); - if (event && event->type == GDK_BUTTON_PRESS) - { - GdkEventButton *event_button = (GdkEventButton *) event; - /* FIXME: should this also use the position func? as the cursor must be on the location - * under the cursor at this point anyway, it might give prettier alignment. - * But might as well be farther from the pointer or otherwise misaligned with the - * pointer, so maybe not. */ - gtk_menu_popup(GTK_MENU(menu), NULL, NULL, NULL, NULL, event_button->button, event_button->time); - } - else - { - if (first) /* always select the first item for better keyboard navigation */ - g_signal_connect(menu, "realize", G_CALLBACK(gtk_menu_shell_select_item), first); - gtk_menu_popup(GTK_MENU(menu), NULL, NULL, goto_popup_position_func, doc->editor->sci, - 0, gtk_get_current_event_time ()); - } - if (event) - gdk_event_free(event); + if (first) /* always select the first item for better keyboard navigation */ + g_signal_connect(menu, "realize", G_CALLBACK(gtk_menu_shell_select_item), first); + gtk_menu_popup(GTK_MENU(menu), NULL, NULL, goto_popup_position_func, doc->editor->sci, + 0, gtk_get_current_event_time ()); } From 1f9bfdf65fd43e80975aeb8eea37a427e379785a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Techet?= Date: Thu, 18 Feb 2016 22:12:56 +0100 Subject: [PATCH 04/10] Set push_in parameter to false This behaves a bit strangely when the list is long and when clicked at the bottom of the screen (the top part of the popup is empty). --- src/symbols.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/symbols.c b/src/symbols.c index 34663d49..a6ce0309 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1892,7 +1892,7 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * *x += pos_x; *y += pos_y + line_height; - *push_in = TRUE; + *push_in = FALSE; } From 848a123f00ba519c47af61c584fece1c5bead1e8 Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Fri, 19 Feb 2016 02:30:37 +0100 Subject: [PATCH 05/10] Improve placement of the goto tag popup --- src/symbols.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index a6ce0309..84c634ad 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1877,7 +1877,7 @@ static guint get_tag_class(const TMTag *tag) } -/* positions a popup below the caret from the ScintillaObject in @p data */ +/* positions a popup at the caret from the ScintillaObject in @p data */ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer data) { ScintillaObject *sci = data; @@ -1887,11 +1887,58 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * gint line_height = scintilla_send_message(sci, SCI_TEXTHEIGHT, line, 0); gint pos_x = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos); gint pos_y = scintilla_send_message(sci, SCI_POINTYFROMPOSITION, 0, pos); + GdkScreen *screen = gtk_widget_get_screen(GTK_WIDGET(menu)); + gint monitor_num; + GdkRectangle monitor; + GtkRequisition req; gdk_window_get_origin(window, x, y); - *x += pos_x; - *y += pos_y + line_height; + *y += pos_y; + + monitor_num = gdk_screen_get_monitor_at_point(screen, *x, *y); + +#if GTK_CHECK_VERSION(3, 0, 0) + gtk_widget_get_preferred_size(GTK_WIDGET(menu), NULL, &req); +#else + gtk_widget_size_request(GTK_WIDGET(menu), &req); +#endif + +#if GTK_CHECK_VERSION(3, 4, 0) + gdk_screen_get_monitor_workarea(screen, monitor_num, &monitor); +#else + gdk_screen_get_monitor_geometry(screen, monitor_num, &monitor); +#endif + + /* put on one size of the X position, but within the monitor */ + if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL) + { + if (*x - req.width >= monitor.x) + *x -= req.width; + else if (*x + req.width > monitor.x + monitor.width) + *x = monitor.x; + } + else + { + if (*x + req.width <= monitor.x + monitor.width) + *x = MAX(monitor.x, *x); + else if (*x - req.width >= monitor.x) + *x -= req.width; + else + *x = monitor.x + MAX(0, monitor.width - req.width); + } + + /* try to put, in order: + * 1. below the Y position, under the line + * 2. above the Y position + * 3. within the monitor */ + if (*y + line_height + req.height <= monitor.y + monitor.height) + *y = MAX(monitor.y, *y + line_height); + else if (*y - req.height >= monitor.y) + *y = *y - req.height; + else + *y = monitor.y + MAX(0, monitor.height - req.height); + *push_in = FALSE; } From a168f69887ce29345d6592b8df566629f014ed7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Techet?= Date: Sat, 20 Feb 2016 00:09:59 +0100 Subject: [PATCH 06/10] Make sure the mouse cursor is out of the popup The x coordinate is now taken from the scintilla caret position. However, when performing ctrl+click, we have to distinguish two cases: 1. the click happens in the second half of a letter - in this case the caret is placed behind the letter and the popup appears behind it - no problem 2. the click happens in the first part of a letter - caret is placed before the letter and the popup appears before the position where the click happened - this means that the mouse cursor is above the popup which causes that the mouse cursor highlights the item at the position of the cursor instead of having the first item in the menu highlighted. The patch calculates offset between caret and the mouse click event position and uses this value to adjust the popup positioning so it's outside the mouse cursor. --- src/symbols.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index 84c634ad..6043e82a 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1888,14 +1888,31 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * gint pos_x = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos); gint pos_y = scintilla_send_message(sci, SCI_POINTYFROMPOSITION, 0, pos); GdkScreen *screen = gtk_widget_get_screen(GTK_WIDGET(menu)); + gint offset_left = 0; + gint offset_right = 0; gint monitor_num; GdkRectangle monitor; GtkRequisition req; + GdkEvent *event; gdk_window_get_origin(window, x, y); *x += pos_x; *y += pos_y; + event = gtk_get_current_event(); + if (event && event->type == GDK_BUTTON_PRESS) + { + GdkEventButton *event_button = (GdkEventButton *) event; + + /* Caret is placed either before or after the letter which was clicked. + * Compute offset between the caret and click position and make sure + * the popup is shown outside the mouse pointer. */ + if (event_button->x >= pos_x && pos + 1 < sci_get_length(sci)) + offset_right = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos + 1) - pos_x; + else if (event_button->x <= pos_x && pos > 0) + offset_left = pos_x - scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos - 1); + } + monitor_num = gdk_screen_get_monitor_at_point(screen, *x, *y); #if GTK_CHECK_VERSION(3, 0, 0) @@ -1913,17 +1930,19 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * /* put on one size of the X position, but within the monitor */ if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL) { - if (*x - req.width >= monitor.x) - *x -= req.width; + if (*x - req.width - offset_left >= monitor.x) + *x -= req.width + offset_left; else if (*x + req.width > monitor.x + monitor.width) *x = monitor.x; + else + *x += offset_right; } else { - if (*x + req.width <= monitor.x + monitor.width) - *x = MAX(monitor.x, *x); - else if (*x - req.width >= monitor.x) - *x -= req.width; + if (*x + req.width + offset_right <= monitor.x + monitor.width) + *x = MAX(monitor.x, *x + offset_right); + else if (*x - req.width - offset_left >= monitor.x) + *x -= req.width + offset_left; else *x = monitor.x + MAX(0, monitor.width - req.width); } From 653990c01137342d5ca1776662594fe976702992 Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Sat, 20 Feb 2016 15:24:45 +0100 Subject: [PATCH 07/10] Pass the actual event button to gtk_menu_popup() In case it's actually useful. Also properly free the GdkEvent returned by gtk_get_current_event(). --- src/symbols.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index 6043e82a..dee8d7f1 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1893,17 +1893,14 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * gint monitor_num; GdkRectangle monitor; GtkRequisition req; - GdkEvent *event; + GdkEventButton *event_button = g_object_get_data(G_OBJECT(menu), "geany-button-event"); gdk_window_get_origin(window, x, y); *x += pos_x; *y += pos_y; - event = gtk_get_current_event(); - if (event && event->type == GDK_BUTTON_PRESS) + if (event_button) { - GdkEventButton *event_button = (GdkEventButton *) event; - /* Caret is placed either before or after the letter which was clicked. * Compute offset between the caret and click position and make sure * the popup is shown outside the mouse pointer. */ @@ -1967,6 +1964,7 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_b GtkWidget *first = NULL; GtkWidget *menu; GdkEvent *event; + GdkEventButton *button_event = NULL; TMTag *tmtag; guint i; @@ -2003,8 +2001,17 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_b if (first) /* always select the first item for better keyboard navigation */ g_signal_connect(menu, "realize", G_CALLBACK(gtk_menu_shell_select_item), first); + + event = gtk_get_current_event(); + if (event && event->type == GDK_BUTTON_PRESS) + button_event = (GdkEventButton *) event; + else + gdk_event_free(event); + + g_object_set_data_full(G_OBJECT(menu), "geany-button-event", button_event, + button_event ? (GDestroyNotify) gdk_event_free : NULL); gtk_menu_popup(GTK_MENU(menu), NULL, NULL, goto_popup_position_func, doc->editor->sci, - 0, gtk_get_current_event_time ()); + button_event ? button_event->button : 0, gtk_get_current_event_time ()); } From c8dd52eb46ec348bb05539124104db910a45c894 Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Sat, 20 Feb 2016 15:35:26 +0100 Subject: [PATCH 08/10] Position the popup at the very click position if any --- src/symbols.c | 54 +++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index dee8d7f1..b07edd1d 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1880,34 +1880,34 @@ static guint get_tag_class(const TMTag *tag) /* positions a popup at the caret from the ScintillaObject in @p data */ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer data) { - ScintillaObject *sci = data; - GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(sci)); - gint pos = sci_get_current_position(sci); - gint line = sci_get_line_from_position(sci, pos); - gint line_height = scintilla_send_message(sci, SCI_TEXTHEIGHT, line, 0); - gint pos_x = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos); - gint pos_y = scintilla_send_message(sci, SCI_POINTYFROMPOSITION, 0, pos); + gint line_height; GdkScreen *screen = gtk_widget_get_screen(GTK_WIDGET(menu)); - gint offset_left = 0; - gint offset_right = 0; gint monitor_num; GdkRectangle monitor; GtkRequisition req; GdkEventButton *event_button = g_object_get_data(G_OBJECT(menu), "geany-button-event"); - gdk_window_get_origin(window, x, y); - *x += pos_x; - *y += pos_y; - if (event_button) { - /* Caret is placed either before or after the letter which was clicked. - * Compute offset between the caret and click position and make sure - * the popup is shown outside the mouse pointer. */ - if (event_button->x >= pos_x && pos + 1 < sci_get_length(sci)) - offset_right = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos + 1) - pos_x; - else if (event_button->x <= pos_x && pos > 0) - offset_left = pos_x - scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos - 1); + /* if we got a mouse click, popup at that position */ + *x = (gint) event_button->x_root; + *y = (gint) event_button->y_root; + line_height = 0; /* we don't want to offset below the line or anything */ + } + else /* keyboard positioning */ + { + ScintillaObject *sci = data; + GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(sci)); + gint pos = sci_get_current_position(sci); + gint line = sci_get_line_from_position(sci, pos); + gint pos_x = scintilla_send_message(sci, SCI_POINTXFROMPOSITION, 0, pos); + gint pos_y = scintilla_send_message(sci, SCI_POINTYFROMPOSITION, 0, pos); + + line_height = scintilla_send_message(sci, SCI_TEXTHEIGHT, line, 0); + + gdk_window_get_origin(window, x, y); + *x += pos_x; + *y += pos_y; } monitor_num = gdk_screen_get_monitor_at_point(screen, *x, *y); @@ -1927,19 +1927,17 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * /* put on one size of the X position, but within the monitor */ if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL) { - if (*x - req.width - offset_left >= monitor.x) - *x -= req.width + offset_left; + if (*x - req.width >= monitor.x) + *x -= req.width; else if (*x + req.width > monitor.x + monitor.width) *x = monitor.x; - else - *x += offset_right; } else { - if (*x + req.width + offset_right <= monitor.x + monitor.width) - *x = MAX(monitor.x, *x + offset_right); - else if (*x - req.width - offset_left >= monitor.x) - *x -= req.width + offset_left; + if (*x + req.width <= monitor.x + monitor.width) + *x = MAX(monitor.x, *x); + else if (*x - req.width >= monitor.x) + *x -= req.width; else *x = monitor.x + MAX(0, monitor.width - req.width); } From b46e183cbc689ba7dd69a21d3575c863d762eea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Techet?= Date: Sat, 20 Feb 2016 16:02:11 +0100 Subject: [PATCH 09/10] Move the popup 1px further so it isn't below mouse pointer Otherwise the mouse pointer highlights a wrong value if the popup is large enough to extend above the mouse pointer. --- src/symbols.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index b07edd1d..14148cd7 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1927,17 +1927,19 @@ static void goto_popup_position_func(GtkMenu *menu, gint *x, gint *y, gboolean * /* put on one size of the X position, but within the monitor */ if (gtk_widget_get_direction(GTK_WIDGET(menu)) == GTK_TEXT_DIR_RTL) { - if (*x - req.width >= monitor.x) - *x -= req.width; + if (*x - req.width - 1 >= monitor.x) + *x -= req.width + 1; else if (*x + req.width > monitor.x + monitor.width) *x = monitor.x; + else + *x += 1; } else { - if (*x + req.width <= monitor.x + monitor.width) - *x = MAX(monitor.x, *x); - else if (*x - req.width >= monitor.x) - *x -= req.width; + if (*x + req.width + 1 <= monitor.x + monitor.width) + *x = MAX(monitor.x, *x + 1); + else if (*x - req.width - 1 >= monitor.x) + *x -= req.width + 1; else *x = monitor.x + MAX(0, monitor.width - req.width); } From 8b61b9f94166e32a921cf839377f0a3b6bfeec3c Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Sun, 28 Feb 2016 13:27:31 +0100 Subject: [PATCH 10/10] Mark the goto popup label as translatable for localized colons It could arguably not be translatable and only `filename:line` display, but as we have already a space on the right of the colon it's not, and allowing translation is not a problem and can give better localization. --- src/symbols.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/symbols.c b/src/symbols.c index 14148cd7..fd6e6e94 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -1979,9 +1979,11 @@ static void show_goto_popup(GeanyDocument *doc, GPtrArray *tags, gboolean have_b gchar *text; if (! first && have_best) - text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); + /* For translators: it's the filename and line number of a tag in the goto-tag popup menu */ + text = g_markup_printf_escaped(_("%s: %lu"), fname, tmtag->line); else - text = g_markup_printf_escaped("%s: %lu", fname, tmtag->line); + /* For translators: it's the filename and line number of a tag in the goto-tag popup menu */ + text = g_markup_printf_escaped(_("%s: %lu"), fname, tmtag->line); 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, NULL);