This adds a filter entry in the keybindings tab in the preferences dialog, similar to the filter entry in the plugin manager dialog.
Remarks: - only the action name is considered for filtering, not the keybindings - the categories are always shown (hiding them on no matching children isn't trivial) - Ctrl-F in the TreeView sets focus to the filter entry - Ctrl-F in the TreeView in the plugin manager dialog sets focus to the filter entry - I also changed the dialog title of the assign keybinding dialog, as suggested in https://github.com/geany/geany/issues/4185
Closes #2848. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/4192
-- Commit Summary --
* Move UTF-8 capable sub string matching from plugin manager to utils * Preferences: add filter entry for keybindings * Plugin Manager: Bind Ctrl-F shortcut to focus the filter entry * Preferences: Retitle assign keybinding dialog title
-- File Changes --
M data/geany.glade (18) M src/plugins.c (56) M src/prefs.c (81) M src/utils.c (48) M src/utils.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/4192.patch https://github.com/geany/geany/pull/4192.diff
@eht16 commented on this pull request.
GtkTreeViewColumn *column;
kbdata->tree = GTK_TREE_VIEW(ui_lookup_widget(ui_widgets.prefs_dialog, "treeview7"));
kbdata->store = gtk_tree_store_new(KB_TREE_COLUMNS, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT); - gtk_tree_view_set_model(GTK_TREE_VIEW(kbdata->tree), GTK_TREE_MODEL(kbdata->store)); + filter_model = gtk_tree_model_filter_new(GTK_TREE_MODEL(kbdata->store), NULL); + gtk_tree_model_filter_set_visible_func( + GTK_TREE_MODEL_FILTER(filter_model), kb_tree_filter_func, kb_filter_entry, NULL); + /* set model to tree view */ + gtk_tree_view_set_model(GTK_TREE_VIEW(kbdata->tree), filter_model); + g_object_unref(filter_model);
Is it correct to unref both objects? It seems to work this way but I'm never sure with unref'ing.
@eht16 pushed 4 commits.
54551a2879e61fc43b59029406a8684f85df69db Move UTF-8 capable sub string matching from plugin manager to utils 140dca74b6a8e7a0ec1674ecfc497fcb3ff81f0f Preferences: add filter entry for keybindings cdedd6ee3fc0a2fb97227e1b9122f92ed82681db Plugin Manager: Bind Ctrl-F shortcut to focus the filter entry 4708f66edf695a8301c00bc129e3d11919eebacc Preferences: Retitle assign keybinding dialog title
Yes!!! I've always wanted this, just didn't get to actually implementing it. (Not tested or reviewed yet, just a big conceptual thumbs up.)
---
While you remember what you are doing in terms of filtering, I think there are also open issues regarding filtering the open documents tab which I think would also be useful too (I don't use it that much myself though).
Also a good opportunity to finally test the LSP plugin :-P.
Setting it up for Geany development is really trivial, just 1. `sudo apt install clangd` 2. `meson setup build` in Geany source directory (so `compile_commands.json` is in the build directory, you don't actually have to build Geany using meson afterwards and can use autotools if you prefer them) 3. Points 2-5 from https://github.com/techee/geany-lsp?tab=readme-ov-file#quick-start
(I still have a feeling that the word "server" scares off many people as everyone imagines some complicated configuration like setting up Apache or whatever and in fact, you typically don't have to set up anything, it's pre-configured in the plugin for most common servers, and the plugin starts/terminates the server for you.)
While you remember what you are doing in terms of filtering, I think there are also open issues regarding filtering the open documents tab which
I might have a look.
(I don't use it that much myself though).
Me neither :(.
Also a good opportunity to finally test the LSP plugin :-P.
Setting it up for Geany development is really trivial, just
1. `sudo apt install clangd` 2. `meson setup build` in Geany source directory (so `compile_commands.json` is in the build directory, you don't actually have to build Geany using meson afterwards and can use autotools if you prefer them) 3. Points 2-5 from https://github.com/techee/geany-lsp?tab=readme-ov-file#quick-start
Thanks, I really should have a look. It'll probably take only a few more months or so :).
Thanks, I really should have a look. It'll probably take only a few more months or so :).
Just saying :-). If nothing else, seeing syntax errors underlined in the editor in the real time without the need to perform actual compilation is a big productivity boost for me.
@eht16 I've just tested the PR briefly but I didn't like that parents not containing any children were still displayed - when filtering for something in the last group, one would have to scroll the tree to see it for the empty groups above.
I think it's hard/impossible to achieve that with the filter function but I tried adding an extra column to the store indicating visibility and then using `gtk_tree_model_filter_set_visible_column()` and it seems to work. I also added `expand_all()` to the result because the collapsed entries weren't very nice.
The patch is below (created very quickly by reusing code from `kb_update()` for the update function, possibly needs improving).
What do you think?
```diff diff --git a/src/prefs.c b/src/prefs.c index 57e95837e..7b8129089 100644 --- a/src/prefs.c +++ b/src/prefs.c @@ -87,7 +87,6 @@ static gboolean kb_find_duplicate(GtkTreeStore *store, GtkWidget *parent, GtkTre static void kb_filter_entry_changed_cb(GtkEntry *entry, gpointer user_data); static void kb_filter_entry_icon_release_cb(GtkEntry *entry, GtkEntryIconPosition icon_pos, GdkEvent *event, gpointer user_data); -static gboolean kb_tree_filter_func(GtkTreeModel *model, GtkTreeIter *iter, gpointer user_data); static gboolean kb_tree_key_press_cb(GtkWidget *widget, GdkEventKey *event, gpointer user_data); static void on_toolbar_show_toggled(GtkToggleButton *togglebutton, gpointer user_data); static void on_show_notebook_tabs_toggled(GtkToggleButton *togglebutton, gpointer user_data); @@ -146,6 +145,7 @@ enum KB_TREE_INDEX, KB_TREE_EDITABLE, KB_TREE_WEIGHT, + KB_TREE_VISIBLE, KB_TREE_COLUMNS };
@@ -276,10 +276,9 @@ static void kb_init_tree(KbData *kbdata, GtkWidget *kb_filter_entry) kbdata->tree = GTK_TREE_VIEW(ui_lookup_widget(ui_widgets.prefs_dialog, "treeview7"));
kbdata->store = gtk_tree_store_new(KB_TREE_COLUMNS, - G_TYPE_STRING, G_TYPE_STRING, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT); + G_TYPE_STRING, G_TYPE_STRING, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT, G_TYPE_BOOLEAN); filter_model = gtk_tree_model_filter_new(GTK_TREE_MODEL(kbdata->store), NULL); - gtk_tree_model_filter_set_visible_func( - GTK_TREE_MODEL_FILTER(filter_model), kb_tree_filter_func, kb_filter_entry, NULL); + gtk_tree_model_filter_set_visible_column(GTK_TREE_MODEL_FILTER(filter_model), KB_TREE_VISIBLE); /* set model to tree view */ gtk_tree_view_set_model(GTK_TREE_VIEW(kbdata->tree), filter_model); g_object_unref(filter_model); @@ -376,14 +375,14 @@ static void kb_init(KbData *kbdata, GtkWidget *kb_filter_entry) { gtk_tree_store_append(kbdata->store, &parent, NULL); gtk_tree_store_set(kbdata->store, &parent, KB_TREE_ACTION, group->label, - KB_TREE_INDEX, g, -1); + KB_TREE_INDEX, g, KB_TREE_VISIBLE, TRUE, -1);
foreach_ptr_array(kb, i, group->key_items) { label = keybindings_get_label(kb); gtk_tree_store_append(kbdata->store, &iter, &parent); gtk_tree_store_set(kbdata->store, &iter, KB_TREE_ACTION, label, - KB_TREE_EDITABLE, TRUE, KB_TREE_INDEX, kb->id, -1); + KB_TREE_EDITABLE, TRUE, KB_TREE_INDEX, kb->id, KB_TREE_VISIBLE, TRUE, -1); kb_set_shortcut(kbdata->store, &iter, kb->key, kb->mods); g_free(label); } @@ -1413,10 +1412,62 @@ static void kb_cell_edited_cb(GtkCellRendererText *cellrenderertext, }
+static void update_visibility(GtkTreeStore *store, const gchar *entry_text) +{ + GtkTreeModel *model = GTK_TREE_MODEL(store); + GtkTreeIter child, parent; + + /* get first parent */ + if (! gtk_tree_model_iter_children(model, &parent, NULL)) + return; + + /* foreach parent */ + while (TRUE) + { + gboolean visible_children = FALSE; + + /* get first child */ + if (! gtk_tree_model_iter_children(model, &child, &parent)) + return; + + /* foreach child */ + while (TRUE) + { + gboolean visible; + gchar *action_name; + + gtk_tree_model_get(model, &child, KB_TREE_ACTION, &action_name, -1); + if (!action_name || EMPTY(entry_text)) + visible = TRUE; + else + visible = utils_utf8_substring_match(entry_text, action_name); + g_free(action_name); + + if (visible) + visible_children = TRUE; + + gtk_tree_store_set(store, &child, KB_TREE_VISIBLE, visible, -1); + + if (! gtk_tree_model_iter_next(model, &child)) + break; + } + + gtk_tree_store_set(store, &parent, KB_TREE_VISIBLE, visible_children, -1); + + if (! gtk_tree_model_iter_next(model, &parent)) + return; + } +} + + static void kb_filter_entry_changed_cb(GtkEntry *entry, gpointer user_data) { GtkTreeModel *filter_model = gtk_tree_view_get_model(GTK_TREE_VIEW(global_kb_data.tree)); + const gchar *entry_text = gtk_entry_get_text(entry); + + update_visibility(global_kb_data.store, entry_text); gtk_tree_model_filter_refilter(GTK_TREE_MODEL_FILTER(filter_model)); + gtk_tree_view_expand_all(global_kb_data.tree); }
@@ -1428,31 +1479,6 @@ static void kb_filter_entry_icon_release_cb(GtkEntry *entry, GtkEntryIconPositio }
-static gboolean kb_tree_filter_func(GtkTreeModel *model, GtkTreeIter *iter, gpointer user_data) -{ - const gchar *key; - gboolean matched; - gchar *action_name; - GtkWidget *kb_filter_entry = user_data; - - // always show categories - if (gtk_tree_model_iter_has_child(model, iter)) - return TRUE; - - gtk_tree_model_get(model, iter, KB_TREE_ACTION, &action_name, -1); - if (!action_name) - return TRUE; - - key = gtk_entry_get_text(GTK_ENTRY(kb_filter_entry)); - if (EMPTY(key)) - return TRUE; - - matched = utils_utf8_substring_match(key, action_name); - g_free(action_name); - return matched; -} - - static gboolean kb_tree_key_press_cb(GtkWidget *widget, GdkEventKey *event, gpointer user_data) { if (event->keyval == GDK_KEY_f && (event->state & GEANY_PRIMARY_MOD_MASK)) ```
I went through the code and looks good to me (note I'm not Colomban though ;-). In addition to the above patch, maybe two additional functionality-related notes:
1. I think it would be good to filter also based on the group name - e.g. if I want all "git changebar" settings and type "git", all its children would be shown. 2. I find it a bit strange that the filter text in the entry is preserved after closing and reopening the Preferences dialog. Wouldn't it be better to clear it?
@techee commented on this pull request.
GtkTreeViewColumn *column;
kbdata->tree = GTK_TREE_VIEW(ui_lookup_widget(ui_widgets.prefs_dialog, "treeview7"));
kbdata->store = gtk_tree_store_new(KB_TREE_COLUMNS, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_INT, G_TYPE_BOOLEAN, G_TYPE_INT); - gtk_tree_view_set_model(GTK_TREE_VIEW(kbdata->tree), GTK_TREE_MODEL(kbdata->store)); + filter_model = gtk_tree_model_filter_new(GTK_TREE_MODEL(kbdata->store), NULL); + gtk_tree_model_filter_set_visible_func( + GTK_TREE_MODEL_FILTER(filter_model), kb_tree_filter_func, kb_filter_entry, NULL); + /* set model to tree view */ + gtk_tree_view_set_model(GTK_TREE_VIEW(kbdata->tree), filter_model); + g_object_unref(filter_model);
I think so (I think I would definitely forget to unref this one).
I tried to add one more `g_object_unref(filter_model);` and it reliably crashes so, experimentally, what you did is the correct balance between refs and unrefs :-)
@eht16 pushed 4 commits.
461d344c5457d6aa3659768d95df48c7c398133a Use Jiri's filter visibility patch 73184d0944bab358e5b2a8aa090daa4170feed20 Connect prefs kb filter entry signals only once 34adcce5ede969c5aaa65aefbfdcbe8456cd9f97 reset prefs kb filter entry text if dialog is opened 3d0d659c596093d5b6c827c938af1fb7b05a8477 Filter also group names in prefs kb filter
Thanks, I really should have a look. It'll probably take only a few more months or so :).
Just saying :-). If nothing else, seeing syntax errors underlined in the editor in the real time without the need to perform actual compilation is a big productivity boost for me.
Well, the latest commits are "geany-lsp" powered :). It works quite nice, so far even without an active project :D.
The patch is below (created very quickly by reusing code from `kb_update()` for the update function, possibly needs improving).
This is awesome and much simpler than I expected. When writing the initial code I shortly thought about using the store to remember the visibility state but then dropped that idea because I imagined the implementation way more complicated. Thanks for pointing me that I was wrong :).
I went through the code and looks good to me (note I'm not Colomban though ;-). In addition to the above patch, maybe two additional functionality-related notes:
1. I think it would be good to filter also based on the group name - e.g. if I want all "git changebar" settings and type "git", all its children would be shown.
Implemented, based on your filter approach this was quite easy.
2. I find it a bit strange that the filter text in the entry is preserved after closing and reopening the Preferences dialog. Wouldn't it be better to clear it?
Oops, didn't notice it. Fixed by clearing the entry text on dialog open and while at it I found also that I connected the signals to the entry each time the dialog was open, not when it was created. Also fixed.
@eht16 commented on this pull request.
{
- GtkTreeModel *filter_model = gtk_tree_view_get_model(GTK_TREE_VIEW(global_kb_data.tree)); - gtk_tree_model_filter_refilter(GTK_TREE_MODEL_FILTER(filter_model));
`gtk_tree_model_filter_refilter` seems not necessary anymore, so I removed it.
@techee commented on this pull request.
/* get first child */ if (! gtk_tree_model_iter_children(model, &child, &parent)) return;
/* foreach child */ - while (TRUE) + while (TRUE && !visible_parent)
`TRUE` can be removed.
@eht16 pushed 1 commit.
6c0ad986be9a447896b6a6a56774da19480a89f4 Remove unnecessary TRUE
@eht16 commented on this pull request.
/* get first child */ if (! gtk_tree_model_iter_children(model, &child, &parent)) return;
/* foreach child */ - while (TRUE) + while (TRUE && !visible_parent)
Oops, thanks, removed.
Well, the latest commits are "geany-lsp" powered :). It works quite nice, so far even without an active project :D.
It's actually surprising (but I can reproduce it too). By default `use_without_project=false` but the server starts even with this set.
Thanks for the "bug report" :-)
Anyway, I've just tested the PR briefly and it seems to do what I'd expect it to do. So apart from the cosmetic `TRUE` comment above (which I can see you have just fixed), the PR looks good to me.
Well, the latest commits are "geany-lsp" powered :). It works quite nice, so far even without an active project :D.
It's actually surprising (but I can reproduce it too). By default `use_without_project=false` but the server starts even with this set.
Thanks for the "bug report" :-)
Oh, I wasn't clear. I had `use_without_project=true` set, so in my case it was not so surprising. I used it this way on purpose. The only drawback is that the project root (after having it detected using `project_root_marker_patterns`), is sent to the server only on initialization. That is, if you switch to another file of the same language in another project root, the server doesn't know the new root directory. Restarting the server helps and might be ok for that use case.
Anyway, I've just tested the PR briefly and it seems to do what I'd expect it to do. So apart from the cosmetic `TRUE` comment above (which I can see you have just fixed), the PR looks good to me.
@vvillenave @advice2020 do you have any chance to test this PR (i.e. compile Geany from source)?
Maybe one more comment - I think it would be good to always update all children, i.e.
```diff diff --git a/src/prefs.c b/src/prefs.c index ec609e5c7..81cc3d623 100644 --- a/src/prefs.c +++ b/src/prefs.c @@ -1444,7 +1444,7 @@ static void kb_tree_update_visibility(GtkTreeStore *store, const gchar *entry_te return;
/* foreach child */ - while (!visible_parent) + while (TRUE) { gboolean visible; gchar *action_name; @@ -1459,7 +1459,7 @@ static void kb_tree_update_visibility(GtkTreeStore *store, const gchar *entry_te if (visible) visible_children = TRUE;
- gtk_tree_store_set(store, &child, KB_TREE_VISIBLE, visible, -1); + gtk_tree_store_set(store, &child, KB_TREE_VISIBLE, visible || visible_parent, -1);
if (! gtk_tree_model_iter_next(model, &child)) break; ```
When you try to search for something like "file ff", it filters out everything. But when you remove the last "f" (so the entry contins "file f"), it should again show all items from the "File" group. It doesn't happen now because children visibility was set to FALSE previously and they don't get re-updated because the while loop is skipped.
Oh, I wasn't clear. I had use_without_project=true set, so in my case it was not so surprising. I used it this way on purpose.
Ah, right, I wasn't sure why it worked for me but it's because I set `project_root_marker_patterns` so the root was auto-located.
The only drawback is that the project root (after having it detected using project_root_marker_patterns), is sent to the server only on initialization. That is, if you switch to another file of the same language in another project root, the server doesn't know the new root directory.
This is specific to `clangd` which unfortunately doesn't support multiple project roots. Many other servers support it - you can find ``` "workspace" : { "workspaceFolders" : { "supported" : true, "changeNotifications" : true } } ``` in their initialize responses and the plugin will automatically add project roots as you open files.
It would be possible to add support for multiple running instances of the same server by which I could overcome the clangd limitation but I kind of hope they eventually add proper `workspaceFolders` support.
So, yeah, restarting is the current workaround (there's a keybinding for it).
@eht16 pushed 1 commit.
45a4e6fec1904de7b032ace4f155fdf7e51eeaff Ensure children are properly (un)filtered in prefs kb
Maybe one more comment - I think it would be good to always update all children, i.e.
diff --git a/src/prefs.c b/src/prefs.c index ec609e5c7..81cc3d623 100644 --- a/src/prefs.c +++ b/src/prefs.c @@ -1444,7 +1444,7 @@ static void kb_tree_update_visibility(GtkTreeStore *store, const gchar *entry_te return; /* foreach child */ - while (!visible_parent) + while (TRUE) { gboolean visible; gchar *action_name; @@ -1459,7 +1459,7 @@ static void kb_tree_update_visibility(GtkTreeStore *store, const gchar *entry_te if (visible) visible_children = TRUE; - gtk_tree_store_set(store, &child, KB_TREE_VISIBLE, visible, -1); + gtk_tree_store_set(store, &child, KB_TREE_VISIBLE, visible || visible_parent, -1); if (! gtk_tree_model_iter_next(model, &child)) break;When you try to search for something like "file ff", it filters out everything. But when you remove the last "f" (so the entry contins "file f"), it should again show all items from the "File" group. It doesn't happen now because children visibility was set to FALSE previously and they don't get re-updated because the while loop is skipped.
Thanks, comitted as is.
I'd squash the last commits before merging.
I'd squash the last commits before merging.
By the way, feel free to squash also the commit performing filtering using the extra column. No credits needed.
Merged #4192 into master.
While you remember what you are doing in terms of filtering, I think there are also open issues regarding filtering the open documents tab which I think would also be useful too (I don't use it that much myself though).
By the way, I tried that and I have it mostly working. It was more complicated though - the filter column update function has to be recursive because the hierarchy is unlimited, but the most annoying (and for me unexpected) part was the need to convert GtkTreeIter between the store representation and the filter store representation because some API (like adding/removing rows) works on top of the store while other API (expanding/collapsing) works on top of the filter store representation.
I need to clean up the code a bit and will post it shortly.
github-comments@lists.geany.org