GNOME Bugzilla – Bug 736522
GbEditorFrame search entry does not allow for specifying match case and other search options
Last modified: 2016-03-12 11:56:50 UTC
We need to add right click menu options to the search entry so that you can choose options for: * match case * match entire word * use regex * wrap around
I started working on it. I thought about other things that would be great to implement, such as: 1 - A combobox which says whether the search is going to be in the current file or in all files of the existing project. 2 - After the implementation of the item above, a button could be added beside the search entry. The action of the button would expand the search entry and display a second entry that'd let us to the called 'find what' and 'replace with'.
I think we are going to be doing something different for "search across project". This bug is strictly for the find within the editor.
(In reply to Christian Hergert from comment #2) > I think we are going to be doing something different for "search across > project". This bug is strictly for the find within the editor. OK. I'll perform just the 'find within the editor'. Thanks
Would it be possible to default to case-insensitive search? I think that's more useful than case-sensitive search, since it allows searching for different representations of the same symbol (i.e. my_namespace_ versus MY_NAMESPACE_).
(In reply to Philip Withnall from comment #4) > Would it be possible to default to case-insensitive search? I think that's > more useful than case-sensitive search, since it allows searching for > different representations of the same symbol (i.e. my_namespace_ versus > MY_NAMESPACE_). FWIW, I absolutely agree with Philip. Case-sensitive search is rarely ever what I want. It's also confusing at first....
Devhelp does some "magic" that detects if an upper case character was typed and does case-sensitive search, otherwise defaulting to insensitive. We do the same in IdePatternSpec. Might be nice to stay consistent throughout Builder.
We do the same in Epiphany. (This is not to say I think it's necessarily a good idea.)
(In reply to Michael Catanzaro from comment #7) > We do the same in Epiphany. (This is not to say I think it's necessarily a > good idea.) We changed this, from now on, search in Epiphany will be case-insensitive.
Created attachment 323539 [details] [review] context menu for search entry I have added right click menu options to the search entry, note that I removed two places of code which unconditionally set the "at-word-boundaries" property of search settings to FALSE. One issue regarding UX to consider, since we have already implemented smart case sensitive search, the "Match Case" switch might be toggled on and off as you type. It seems to only add confusion to the user, maybe we can default to case insensitive search.
Review of attachment 323539 [details] [review]: keep up the great work! ::: libide/editor/ide-editor-frame.c @@ -667,0 +655,9 @@ +ide_editor_frame__search_populate_popup (IdeEditorFrame *self, + GtkWidget *popup, + GdTaggedEntry *entry) ... 6 more ... We should use the menu merging infrastructure I put into IdeApplication here. Not only does it save creating a bunch of menu items manually, it allows plugins to extend the menu and add new actions. Something like: menu = ide_application_get_menu_by_id (IDE_APPLICATION_DEFAULT, "ide-editor-frame-search-menu"); gtk_menu_shell_bind_model (GTK_MENU_SHELL (popup), menu); This will require changing the property bindings to stateful GActions though (add them to ide-editor-frame-actions.c)
(In reply to Fangwen Yu from comment #9) > One issue regarding UX to consider, since we have already > implemented smart case sensitive search, the "Match Case" switch > might be toggled on and off as you type. It seems to only add > confusion to the user, maybe we can default to case insensitive > search. I think defaulting to case-insensitive would be fine. The previous guessing code was more of a stop-gap IMO. The other option would be to keep the case-guessing code, and use more of a "tri-state" instead of boolean (-1, 0, 1).
(In reply to Christian Hergert from comment #11) > I think defaulting to case-insensitive would be fine. The previous > guessing code was more of a stop-gap IMO. This is my recommendation, for consistency with other GNOME applications. (We could change Devhelp too.)
Created attachment 323657 [details] [review] context menu for the search entry
(In reply to Christian Hergert from comment #10) > We should use the menu merging infrastructure I put into IdeApplication > here. Not only does it save creating a bunch of menu items manually, it > allows plugins to extend the menu and add new actions. > > Something like: > > menu = ide_application_get_menu_by_id (IDE_APPLICATION_DEFAULT, > "ide-editor-frame-search-menu"); > gtk_menu_shell_bind_model (GTK_MENU_SHELL (popup), menu); I have adapted the code and re-uploaded a patch, but the menu "merging" seems not working, what "gtk_menu_shell_bind_model (popup, menu)" does is replacing "popup" with "menu", so I lost the original default Copy/Cut/Paste, haven't figured out how to bring them back to the context menu, their functionality can be accessed with keyboard shortcuts though.
Created attachment 323724 [details] [review] context menu for search entry
I finally get all the menu items working, now its behavior is exactly the same with gedit. The code seems to be a little repetitive, adding all the Cut/Copy/Paste stuff manually makes the code longer, but it does help seperate action from view. The code before is more of functional programming style, and it's more clean in my opinion.
Review of attachment 323724 [details] [review]: Despite "more code", it doesn't seem unreasonable to me. If we can use GPropertyAction, I'd like to do that. If we can't for some technical reason I've missed, I'd be fine pushing this as is. ::: libide/editor/ide-editor-frame-actions.c @@ -98,2 +98,4 @@ } +static void +ide_editor_frame_actions_change_case_sensitive (GSimpleAction *action, I have a hunch we could use GPropertyAction for some of these. Have you see GPropertyAction? It lets you turn a GObject property into a stateful action. That would be something like: g_property_action_new ("case-senstitive", search_settings, "case-sensitive"); and then add that to the GSimpleActionGroup with g_action_map_add_action().
Created attachment 323742 [details] [review] context menu for search entry, with GPropertyAction
Looks good! Thanks! The duplication of cut/copy/paste is not ideal, but not the end of the world. Maybe that is something we can look at fixing while improving our GActionGroup helpers during the 3.22 cycle.