Created attachment 285656 [details] [review]
Patch[1/2]: new gtk_source_style_equal() function

Created attachment 285657 [details] [review]
Patch[2/2]: Custom style support for text matches

Review of attachment 285657 [details] [review]:

Why do you need a custom style only for the search matches?
You can create a new style scheme. The new style scheme can be based on another one, with the parent-scheme attribute.
Patches are also welcomed to improve the colors of the existing style schemes, if a color doesn't look nice.

Ideally the style schemes should be converted to CSS, so it would maybe be easier to customize the style in an application, or by the user.

Anyway, some comments below for the patch.

::: gtksourceview/gtksourcesearchcontext.c
@@ +390,3 @@
 	if (style == NULL)
 	{
+		g_warning ("No custom style nor \'search-match\' style available.");

No need to escape the single quotes, since the string is delimited by double quotes.

::: gtksourceview/gtksourcesearchsettings.c
@@ +266,3 @@
 {
 	self->priv = gtk_source_search_settings_get_instance_private (self);
+	self->priv->match_style = NULL;

The priv struct is initialized with 0's, like g_slice_new0() does.

@@ +363,3 @@
+
+	if (settings->priv->match_style != NULL && match_style != NULL &&
+	    gtk_source_style_equal (settings->priv->match_style, match_style))

Just comparing the pointers should be sufficient.
gtk_source_style_equal() is not really needed, it's not a big problem if the notify signal is sent uselessly.

Review of attachment 285656 [details] [review]:

As I said in the review for the other patch, a simple pointer comparison should be enough, so gtk_source_style_equal() is not really useful.

Created attachment 285717 [details]
Quick highlight + regular search highlight

The goal is to use the new (powerfull) search mechanism not only for regular Ctrl+F search but also, for exemple here, for quick/smart hightlight of selected text (cf. attachment).

This requires to be able to specify a style for every search action performed. Currently, GtkSourceSearchContext can only use the "search-match" theme-defined style, and this is hardcoded. If this style could be changed for any other one, plugins could re-use the search mechanism for other purposes than just regular search (quick selection hightlight is one, text bookmarking could be another).

"Custom style" was probably wrong, "different style" is better. GtkSourceStyle cannot really be customised, due to its implementation. The object is basically just a carrier if you can't access its public members (and you need gtksourcestyle-private.h for that). But that is a different problem...

Created attachment 285718 [details] [review]
Patch[1/1]: allow caller to choose a style for text matches

Review of attachment 285718 [details] [review]:

Alright. Indeed be able to provide a different GtkSourceStyle is a good idea.

But the appearance settings should be in the SearchContext, not SearchSettings, because different buffers can have different style schemes, and you may want to choose a different GtkSourceStyle depending on the style scheme. See the paragraph beginning by "The search occurrences are highlighted by default." in the description of the SearchContext class (and this paragraph needs to be updated. Maybe add an explanation that different buffers can have different style schemes, that's why the appearance settings are in SearchContext).

Also, the API should be able to be extended in the future to add the concept of "current match", i.e. be able to provide a different style for the current match. But I think there is no problem with the name "match-style". For the current match, "current-match-style" can be added in the future.

::: gtksourceview/gtksourcesearchsettings.c
@@ +362,3 @@
+
+	if (settings->priv->match_style != NULL && match_style != NULL &&
+	    settings->priv->match_style == match_style)

Now only the second line of the condition is enough.

@@ +378,3 @@
+	else
+	{
+		settings->priv->match_style = gtk_source_style_copy (match_style);

I think a copy is not needed, just call g_object_ref().

Created attachment 285744 [details] [review]
 Patch[1/1]: allow caller to choose a style for text matches 

Agreed, should have read that more carefully I guess. I made the documentation modification, let me know what you think.

New patch is moving the style property to SearchContext. About the "match-style" name, "highlight-style" may sound more natural but "current-highlight-style" may be a bit to long... Actually anything with "*-style" is good to me.

Comment on attachment 285744 [details] [review]
 Patch[1/1]: allow caller to choose a style for text matches 

The patch is almost ready to be merged (after the freeze, for the next development cycle). There are just a few details that can be improved.

>+ * have a different text style associated. Use
>+ * gtk_source_search_context_set_match_style() in order to specify the
>+ * #GtkSourceStyle to be applyied on a #GtkSourceSearchContext search result.

I prefer:
"to specify the #GtkSourceStyle to apply on search matches."

>+ * settings should be tied to one, and only one buffer, as different buffers can
>+ * have different theme scheme associated (a #GtkSourceSearchSettings object

"style scheme", not theme scheme.

>+		case PROP_MATCH_STYLE:
>+			gtk_source_search_context_set_match_style (search, GTK_SOURCE_STYLE (g_value_get_object (value)));
>+			break;

g_value_get_object() returns a gpointer, so no need to use GTK_SOURCE_STYLE.
The object type will be checked by gtk_source_search_context_set_match_style().

> /**
>+ * gtk_source_search_context_get_match_style:
>+ * @search: a #GtkSourceSearchContext.
>+ *
>+ * Returns: (transfer none): the #GtkSourceStyle to be applied on matching text
>+ * occurrences.

I prefer:
"to apply on search matches."

(make the same change for the set() function too).

>+ * Set the style to be applied on matching text occurrences. If @match_style is
>+ * %NULL, default theme's scheme 'match-style' will be used.
>+ * Use gtk_source_search_context_set_highlight() for managing the occurrences
>+ * highlighting activation state.

"To enable or disable the search highlighting, use gtk_source_search_context_set_highlight()."

>+	if (match_style == NULL)
>+	{
>+		search->priv->match_style = NULL;
>+	}
>+	else
>+	{
>+		search->priv->match_style = g_object_ref (G_OBJECT (match_style));
>+	}

g_object_ref() takes a gpointer, so no need to use G_OBJECT.
The code can be simplified like this:

search->priv->match_style = match_style;

if (match_style != NULL)
{
  g_object_ref (match_style);
}

>+void			 gtk_source_search_context_set_match_style		(GtkSourceSearchContext *search,
>+										 const GtkSourceStyle   *match_style);

For a struct like GtkTextIter we use const, but for a GObject we usually don't use const.
(one reason is probably because when g_object_ref() is called, we change the ref count stored in the GObject, so technically we modify the GObject instance struct).

Created attachment 285824 [details] [review]
Patch[1/1]: allow caller to choose a style for text matches

Review of attachment 285824 [details] [review]:

Thanks, I'll push the commit once the gnome-3-14 branch is created.

Pushed:
https://git.gnome.org/browse/gtksourceview/commit/?id=6d2d56dbe984af23baf8953b5590b583a9e1c2b5