Adds enhancement to save search history after quitting and load on next start. Fixes #1666. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1745
-- Commit Summary --
* Remember search history (#1666)
-- File Changes --
M src/search.c (102)
-- Patch Links --
https://github.com/geany/geany/pull/1745.patch https://github.com/geany/geany/pull/1745.diff
AFAICT nothing stops the search history growing to infinity (or OOM anyway), save history, load history, add some more, save history ...
Past term searches are limited using the function ui_combo_box_add_to_history().
ui_combo_box_add_to_history() only limits what's added to the combo box, and only if the len param is <= 0. The underlying storage (including geany.conf) is not limited.
The underlying storage should be limited to 10 as well (or maybe 15?)
b4n commented on this pull request.
@@ -2297,3 +2337,59 @@ void search_find_again(gboolean change_direction)
toolbar_get_widget_child_by_name("SearchEntry"), (result > -1)); } } + + +static void load_combo_box_history(GtkWidget *combo_entry, gchar **recent_terms) +{ + guint i, len = 0; + + if (recent_terms != NULL) + { + len = g_strv_length(recent_terms);
Implementation note: `g_strv_lenght()` is fairly costly (not that it actually really matters here, but well), and not really necessary here. You could simply do:
```C for (; recent_terms != NULL && *recent_terms != NULL; recent_terms++) { ui_combo_box_add_to_history(GTK_COMBO_BOX_TEXT(combo_entry), *recent_terms, 0); }
b4n commented on this pull request.
- if (count == 0)
+ { + *recent_terms = NULL; + return; + } + + *recent_terms = g_new0(gchar*, count + 1); + + if (gtk_tree_model_get_iter_first(model, &iter)) + { + do + { + gtk_tree_model_get(model, &iter, 0, &combo_text, -1); + if (*combo_text != 0) + { + (*recent_terms)[i] = g_strdup(combo_text);
you could avoid having to duplicate the `combo_text` if you freed it conditionally:
```C if (NZV(combo_text)) { (*recent_terms)[i] = combo_text; } else { g_free(combo_text); } ```
b4n commented on this pull request.
- g_strfreev(*recent_terms);
+ + if (count == 0) + { + *recent_terms = NULL; + return; + } + + *recent_terms = g_new0(gchar*, count + 1); + + if (gtk_tree_model_get_iter_first(model, &iter)) + { + do + { + gtk_tree_model_get(model, &iter, 0, &combo_text, -1); + if (*combo_text != 0)
Why not saving empty elements? It would make a lot of sense for replacement strings: it's totally reasonable to replace with an empty string to remove the searched pattern.
Why not saving empty elements? It would make a lot of sense for replacement strings: it's totally reasonable to replace with an empty string to remove the searched pattern.
Well OK as the empty string would be the default anyway it might not be useful in practice, so maybe ignore that part. Though, I'm still curious why you went to the trouble not to save those?
@Petros-Tsol pushed 1 commit.
38e7eb0 Remember search history(#1666) reviewed
I followed your recommendations except from macro NZV which has a deprecation comment so I used EMPTY instead. As for your question about the empty term, it was just a thought of the moment and I didn't think over.
Just tested this right now. Works well for me. I've still spotted an issue in that the history is filled in a strange order (seems to be once last item, then first, then last and so on?!). Probably not related to this patch but, since it is now beeing saved, the history will sooner or later be full (and keep beeing full forever) and this strange order has the side effect of deleting an item that is not necessarily the older when inserting a new one… which is not good. About infinite growth, yes it seems currently clamped by the combobox history mechanism, but I second the fear of elextr: should it really rely on some external UI stuff that may change / break anytime later to be safe on Geany side?
github-comments@lists.geany.org