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 315055 - PATCH: Change the backgound color of lines containing markers
PATCH: Change the backgound color of lines containing markers
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
unspecified
Other All
: High enhancement
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2005-09-01 17:50 UTC by Paolo Maggi
Modified: 2014-02-15 12:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch v.1 (20.20 KB, patch)
2005-09-01 17:51 UTC, Paolo Maggi
none Details | Review
new patach (15.93 KB, patch)
2008-08-03 07:31 UTC, Yevgen Muntyan
none Details | Review
patch (16.02 KB, patch)
2008-08-04 16:38 UTC, Yevgen Muntyan
none Details | Review

Description Paolo Maggi 2005-09-01 17:50:41 UTC
The patch I'm going to attach is the first version of a patch aiming at
implementing the possibility to change the background color of lines containing
markers.

It is possible to set the background color that has to be used for a given
marker type using the gtk_source_buffer_set_marker_background_color (note that
it is not a view property as for the gtk_source_view_set_marker_pixbuf, this is
due to the gact we are using the paragraph-background text tags to implement
this feature).

The patch probably needs some review before being committed. BTW, I have tested
it and it seems to work very well.

The only problem I have seen are the following:
- it doesn't highlight the last line if the line is completely empty (this is
due to bug #313718)
- when copying and pasting a line containing a marker the background color is
pasted to (this is due to bug #79868) -> this is a blocker

From the code reviewing point of view the most important functions are:
- update_markers_line: it is needed to update the tags in a specified region
(the used algorithm is commented in the code)
- gtk_source_buffer_remove_all_marker_tags: it does remove all the marker tags
from a given region. It is a quite complex function, but since it is a slighty
modified copy of an existing well tested function
(gtk_source_buffer_remove_all_source_tags), I'm quite confident of its correctness.

Note that:
- I have not tested gtk_source_buffer_move_marker
- I have not tested if changing the background color of an existing marker works
as expected.
- I have not checked it for memory leaks.
- I don't think it introduces big performance problems, but we should check.

This patch also show some existing problems in gtksourceview we should try to check:
- why GtkSourceMarker does not simply extends GtkTextMark? There is probably a
reason but I don't remember it. Gustavo: do you remember it?
- I don't like very much the way the markers are moved while deleting text. For
example if the first two chars of a marked line are deleted, the marker is
deleted too.
- I has not been able to use the marker_updated signal since it does have a
GtkSourceMarker argument
- We should review when/why the marker_updated signal is emitted
- gtk_source_marker_get_marker_type should probably return a const gchar * (but
I think it does not worth breaking API for fixing this problem).
Comment 1 Paolo Maggi 2005-09-01 17:51:54 UTC
Created attachment 51671 [details] [review]
Patch v.1

Note that it still lacks documentation and ChangeLog entry.
Comment 2 Yevgen Muntyan 2008-08-03 07:31:38 UTC
Created attachment 115767 [details] [review]
new patach

New patch. Draw rectangles, like for current line.
Comment 3 Paolo Borelli 2008-08-04 07:17:59 UTC
The patch looks mostly good to me, but I think the public function should take a "const GdkColor", like gtk_cell_view_set_background_color or like our _gtk_source_style_scheme_get_current_line_color (even if it this one is private).

Probably we should also have a getter (e.g. if someone implemnts an ui to let the user pick the color he doesn't have to store the category-color map himself)


Also do not forget to add the function to gtk-doc
Comment 4 Yevgen Muntyan 2008-08-04 16:38:47 UTC
Created attachment 115837 [details] [review]
patch

Updated patch. Setter takes const GdkColor*, getter takes out-GdkColor* and returns gboolean. Will commit in a while if there are no objections.
Comment 5 Yevgen Muntyan 2008-08-04 18:39:34 UTC
2008-08-04  Yevgen Muntyan  <muntyan@tamu.edu>

	Bug 315055 – Change the backgound color of lines containing markers
	New API: gtk_source_view_set_mark_category_background(),
	gtk_source_view_get_mark_category_background(), similar to methods
	for priority and pixbuf.

	* gtksourceview/gtksourceview.c: replaced GtkSourceViewPrivate::current_line_gc
	with current_line_color and current_line_color_set;
	added MarkCategory::background and MarkCategory::background_set;
	removed gtk_source_view_unrealize() since current line GdkGC is removed;
	(source_mark_updated_cb): call gtk_widget_queue_draw() instead of invalidating
	only margin window;
	(gtk_source_view_get_lines): added an array argument to get line heights,
	to use in line background drawing code;
	(gtk_source_view_paint_line_background), (gtk_source_view_paint_marks_background):
	new functions;
	(gtk_source_view_expose): use those; do not draw current line background if
	the widget is insensitive;
	(mark_category_set_background), (gtk_source_view_set_mark_category_background),
	(gtk_source_view_get_mark_category_background): new functions;
	(gtk_source_view_set_mark_category_pixbuf), (gtk_source_view_set_mark_category_priority):
	call gtk_widget_queue_draw() here;
	(update_current_line_color), (gtk_source_view_realize), (gtk_source_view_update_style_scheme):
	only update current line color, no need to fiddle with GdkGC anymore.
	* gtksourceview/gtksourceview.h: gtk_source_view_set_mark_category_background(),
	gtk_source_view_get_mark_category_background().
	* tests/test-widget.c: set background color for marks.