After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726981 - Improve the search and replace API
Improve the search and replace API
Status: RESOLVED OBSOLETE
Product: gtksourceview
Classification: Platform
Component: General
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-24 17:05 UTC by Sébastien Wilmet
Modified: 2021-07-05 11:00 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sébastien Wilmet 2014-03-24 17:05:51 UTC
This is too late for GtkSourceView 3, but for GtkSourceView 4 we can consider breaking the API for improving it.

1. One issue that I see is that GtkSourceBuffer doesn't own the SearchContext. So all the owners of the SearchContext are external to GtkSourceView. In GtkSourceView there are two weak references in the two directions between the Buffer and the SearchContext.

See bug #726408 (especially comment 13) for a problem in gedit: the gedit_document_{set,get}_search_context() functions are strange to use. Those functions are needed for the "Clear Highlight" action (destroy the search context so the search highlighting is cleared).

A solution is to have gtk_source_buffer_attach_search_context() and detach(), or something like that. With a function to clear the search highlighting, which would detach the search contexts that are highlighted. And/or a function to get all the search contexts attached to the buffer.

Another idea is to remove gtk_source_search_context_new(), make gtk_source_search_context_get_type() private and add a gtk_source_buffer_create_search_context() function. But I don't see how it would resolve the gedit problem.

2. Another minor issue is that the GtkSourceSearchContext:settings property is not construct-only. It is an advantage to be able to change the settings object. But at some other places it is a disadvantage because you have to connect to the notify::settings signal. Maybe a better solution is to make the settings property construct-only, and when another settings object is required, we create another SearchContext. The SearchSettings properties can anyway be modified.

I think a little more thoughts on this API can not hurt.
Comment 1 Sébastien Wilmet 2016-06-04 12:34:24 UTC
For the weak references, if the Buffer has a strong ref to the SearchContext, it is anyway safer to have a weak ref on the other side (from SearchContext to Buffer).

If the SearchContext has a strong ref to the Buffer, the Buffer (or a subclass of it) cannot store the SearchContexts. But the Buffer is the obvious place to store the SearchContexts. So the annoyance of having the weak ref in SearchContext cannot be circumvented.
Comment 2 Sébastien Wilmet 2016-09-11 10:52:03 UTC
I think it's fine that GtkSourceBuffer doesn't own the SearchContexts.

gedit_document_{set,get}_search_context() is ugly, but there is a better solution:
- Add a GtkSourceSearchSettings:entry-text property that stores what the GtkEntry should contain.
- Add back gedit_document_set_search_text() that sets the :entry-text and :search-text properties (or creates a new SearchContext & SearchSettings if NULL). The search entry can then update its contents. And the plugin doesn't need to create its own SearchContext.

But :entry-text and :search-text would be redundant. A boolean property can be added to unescape :entry-text. And for the set_entry_text() function, there can be a boolean parameter to escape the text. That way, the utils functions to escape/unescape the search text can be deprecated/removed. And maybe the :search-text property can also be deprecated/removed, unless we want to still allow low-level control (to use other escape/unescape functions) while still benefiting from the :entry-text property.
Comment 3 Sébastien Wilmet 2016-09-11 11:02:54 UTC
What is described in Comment 2 can be done in a backward-compatible way.

For GSV 4, the only thing to do is to make the GtkSourceSearchContext:settings property construct-only.
Comment 4 Sébastien Wilmet 2016-10-19 13:24:13 UTC
(In reply to Sébastien Wilmet from comment #3)
> For GSV 4, the only thing to do is to make the
> GtkSourceSearchContext:settings property construct-only.

Done in commit 77906cf17d5c4905c8efb00f325253d7f772a18e.
Comment 5 GNOME Infrastructure Team 2021-07-05 11:00:49 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gtksourceview/-/issues/

Thank you for your understanding and your help.