GNOME Bugzilla – Bug 701466
Higher level API for the search of text
Last modified: 2014-01-09 17:26:25 UTC
With this higher level API, the search matches would be highlighted (asynchronously), and some information would be reported (asynchronously too). A good place, in my opinion, to add this API is the GtkSourceView class. Obviously the implementation will deal with the buffer, so the search matches will be visible in all views that share the same buffer. But in an ideal future, it will be possible to apply a GtkTextTag (or equivalent) to a single view. Here is a proposition for the API: > void > gtk_source_view_set_search (gchar *str, > GtkTextSearchFlags flags, > GtkTextIter *start, > GtkTextIter *end); If @start is NULL, it means the start of the buffer, and if @end is NULL, it means the end of the buffer. The purpose of these parameters is to search only in a region of text. > void > gtk_source_view_clear_search (void); Another possibility is to pass NULL to @str on the first function. But the other parameters can be misleading. For example, what happens if the @start and @end are different when we clear the search? Having another function avoids this confusion. > The GtkSourceView:search-occurrences-count property The number of occurrences. -1 if the search is cleared. > The GtkSourceView:search-occurrence-num property On which occurrence the cursor is located. -1 if the cursor is not in a search occurrence. The selected occurrence should be highlighted differently. Did I miss something?
makes sense... but how do you specify to move forward or backward?
(Side note: in the above functions, I forgot the first parameter of type GtkSourceView*, obviously). The low level API in GtkTextIter can be used to move forward or backward. But we can add convenience functions in gsv: > gboolean > gtk_source_view_backward_search (GtkSourceView *view); > gboolean > gtk_source_view_backward_cycle_search (GtkSourceView *view, > gboolean *has_cycled); Returns TRUE if another search occurrence is selected. FALSE if we didn't move (no occurrences, only one occurrence, or at the first occurrence if we don't cycle). Same functions for the forward search. Instead of have four functions, we can have more generic function(s), with flags: CYCLE, BACKWARD/FORWARD. But it becomes less convenient. ---------- What could be useful too is to get the bounds of a search occurrence: > gboolean > gtk_source_view_get_search_occurrence_bounds (GtkSourceView *view, > guint occurrence_num, > GtkTextIter *begin, > GtkTextIter *end); Returns FALSE if @occurrence_num doesn't exist. We can also add convenience functions for the "replace" and "replace all" actions. I've thought also about a special iterator to traverse the search occurrences. But get_search_occurrence_bounds() should be sufficient (or the functions for replace and replace all).
I think I'd really start with a simpler API, what we really want is a GtkSourceView:search-occurrences-count and simply go forward and go backward... do we have use cases for more api than that?
- set_search() is needed to highlight the matches. - clear_search() is needed to avoid confusion if the first function is used for clearing the search. - The search-occurrence-num property is useful to display something like "3 of 10" in the search entry (or elsewhere). search-occurrence-num tells on which occurrence the cursor is located. The occurrence could be entirely selected (like in gedit, if I'm right). But if the user can continue to edit the document while the search entry is still visible (and thus the search matches are still visible too), when the user clicks inside an occurrence, this occurrence becomes highlighted differently, and the "3 of 10" in the search entry is updated accordingly. This latter behavior is used by MonoDevelop. - backward_search() and forward_search(): OK. - backward_cycle_search() and forward_cycle_search(): there is often an option in applications to cycle through the document (continue at the top/bottom). - get_search_occurrence_bounds(): without this function, it is difficult to get the bounds of an occurrence. - For the convenience functions for the "replace" and "replace all" actions, it is the same idea as for the other functions, i.e. simplify at most as possible the implementation of the search and replace in an application. Almost the only missing thing would be the UI (and we can even provide a special text entry in gsv, with the integrated "3 of 10", "not found", etc.).
For the backward_search() and backward_cycle_search(), we could add only the former, with a property GtkSourceView:cycle-search. We don't have the @has_cycled parameter, but the :search-occurrence-num is there.
Some random comments (not in a really coherent order, it's saturday morning ;) ) - I am not 100% convinced we want to bind the "current" match with the cursor position - Since the API is async, do we get a signal when searching is done? what happens when the content is edited and the number of matches changes? - whether to put ths in view or buffer probably needs some more thinking: I agree with the idea that ideally this would be per view, but given the current state of things buffer could be a more natural place - this search is stateful, so we need properties to query the search pattern and search flags - we need to also have replace and replace_all (which is important because it needs to be optimized to avoid spending time updating "current-match-num" all the time). This can be done in a second step, let's just be aware of it.
(In reply to comment #6) > - I am not 100% convinced we want to bind the "current" match with the cursor > position How would the current match be defined, then? > - Since the API is async, do we get a signal when searching is done? Yes, we can add a signal. It can be useful to stop a spinner, for example. > what happens when the content is edited and the number of matches changes? The properties are updated when the processing is done. For example, when we call set_search(), we update the value of search-occurrences-count only when we have the total number of occurrences. > - whether to put ths in view or buffer probably needs some more thinking: I > agree with the idea that ideally this would be per view, but given the current > state of things buffer could be a more natural place Moreover, we don't really know if the view would be a good place in the ideal future. Maybe there will be another class. > - this search is stateful, so we need properties to query the search pattern > and search flags Right. > - we need to also have replace and replace_all (which is important because it > needs to be optimized to avoid spending time updating "current-match-num" all > the time). This can be done in a second step, let's just be aware of it. Yes, so this isn't only for convenience.
Created attachment 248538 [details] [review] Higher-level asynchronous search and replace API And the implementation, the documentation, some unit tests, a test for the performances, a mini program with a basic UI, and utilities functions.
Review of attachment 248538 [details] [review]: This looks really good (I reviewed the api and only glanced at the implementation in gtksourcesearch.c) Some comments below... ::: gtksourceview/gtksourcebuffer.c @@ +108,3 @@ + * gtk_source_buffer_forward_search_async() for the asynchronous version. + * The backward search is done similarly. To replace a search match, or all + * </para> A couple of questions here (note I have not looked at the patch yet, which makes it a good test for the docs :) 1 - what does search_replace work on? the current match? the first match after the insert mark? does it require to be placed at the occurrence or will it move? 2 - is replace all blocking or async? replacing on a big doc could take a lot of time and may be worth cancelling... @@ +2967,3 @@ + * @match_start: return location for start of match, or %NULL. + * @match_end: return location for end of match, or %NULL. + * Enables or disables the wrap around search. If @wrap_around is %TRUE, the random thought: we could set a specific error when the end of the buffer is reached... think of how to implement an UI that notifies you about reaching the end of file and asking if you want to start from the top. Not sure how to map it to the sync api though... Which brings me to: do we really want sync and async? I know gio sets a precedent, but I think here we should guide the user to the proper async api... ::: gtksourceview/gtksourcesearch.c @@ +2107,3 @@ + } + + gtk_text_buffer_get_start_iter (search->priv->buffer, &iter); I think we should freeze the highlighting while doing the replace...
(In reply to comment #9) > 1 - what does search_replace work on? the current match? the first match after > the insert mark? does it require to be placed at the occurrence or will it > move? The insert and selection_bound marks are not taken into account. When refering to match, the two iters are needed. But in the future this can change, gsv can know the "current match", and we could pass NULL to the iters to refer to the current match. > 2 - is replace all blocking or async? replacing on a big doc could take a lot > of time and may be worth cancelling... It is blocking. I can add an async version though. But when the replace all is cancelled, what should be the correct behavior? If half of the buffer is replaced, it is not in a coherent state. And undoing all the replacements take the same amount of time (and should be blocking). > random thought: we could set a specific error when the end of the buffer is > reached... think of how to implement an UI that notifies you about reaching the > end of file and asking if you want to start from the top. If an application wants to do that, it can disable the wrap around setting, and do the work manually. If the forward_search() returns false, notify the user, and take the start iter if the user wants to continue the search from the top. > Which brings me to: do we really want sync and async? I know gio sets a > precedent, but I think here we should guide the user to the proper async api... Yes, I can recommend to use the async API, but a synchronous API can be useful if it's always a small document. > ::: gtksourceview/gtksourcesearch.c > @@ +2107,3 @@ > + } > + > + gtk_text_buffer_get_start_iter (search->priv->buffer, &iter); > > I think we should freeze the highlighting while doing the replace... Yes, I forgot that.
Pushed to the master branch, yay! The API can still change until the freeze.