GNOME Bugzilla – Bug 744179
Allow draw-whitespace for tagged text
Last modified: 2015-10-11 09:55:08 UTC
In Meld, it would be interesting to have the ability to have a "draw-whitespace" property for a GtkTextTag (or similar functionality). Currently, inline highlighting of differences between diff chunks is done with GtkTextTags that apply a themed background to characters that differ between lines. When these differences include whitespace characters, it can be difficult to tell what the difference actually is. It would be very cool to be able to draw whitespace characters *only* for differing parts of lines, as this would let the inline highlighting easily show the user that e.g., on the left you have a tab and on the right it's eight spaces, or the left has a CRLF and the right has a LF. Being able to do this only for differing sections means that it's much less distracting to the user if they're not used to having visible whitespace. I wasn't sure whether to file this against gtksourceview or GTK+, so let me know if you think it belongs elsewhere. Related bugs are bug 683678 and bug 737796, but I'm pretty sure neither will actually let us do what we want here.
Interesting use case. Indeed a GtkTextTag (or subclass) property would work fine. Currently the space drawing is done in GtkSourceView, but ideally it should be done in GtkTextView/pango, see bug #721015. We can have in parallel the API proposed in bug #683678 and a GtkTextTag property. But the former should not use the latter API, because inserting lots of tags in the text buffer is not a good idea (it uses more memory, it's slower, etc). So a first step is to have those both APIs in GtkSourceView, and longer-term move that to GtkTextView/pango.
I'm working on it (at my day job). The plan is to create a GtkSourceTag class, a subclass of GtkTextTag, to add the boolean property draw-spaces which takes precedence over the GtkSourceView:draw-spaces property. I need it for a CSV editor where the columns are aligned. Virtual spaces are added for the alignment, but I don't want to draw spaces in the virtual spaces, only for the real spaces present in the file. (virtual spaces are added as normal spaces in the GtkTextBuffer, I still need to write a utility class to (not) take into account the virtual spaces for various features).
Almost done: https://github.com/swilmet/gtksourceview/commits/wip/space-drawing-tag and it works fine. I'll attach the patches here when a test is written in GSV and the doc is updated. Currently I don't know exactly how to test it in test-widget, probably the simplest is to move the space drawing tests in another test.
To be able to subclass GtkTextTag cleanly and send the GtkTextTagTable::tag-changed signal, bug #755416 in GTK+ needs to be fixed.
Created attachment 311873 [details] [review] Add an empty GtkSourceTag class GtkSourceTag is a subclass of GtkTextTag. It'll be useful to add properties to it. gtk_source_buffer_create_tag() is also implemented, for convenience and consistency with how GtkTextTags are usually created.
Created attachment 311874 [details] [review] tag: add a property to draw spaces
Created attachment 311876 [details] [review] Add test-space-drawing to test the GtkSourceTag property It isn't a unit test, just a simple GTK+ app. The GtkSourceView's draw-spaces property can be changed (as a boolean) and the GtkSourceTag's draw-spaces-set and draw-spaces properties can be changed individually. The test shows that there is a problem when changing the GtkSourceTag properties, the view is not redrawn.
Created attachment 311877 [details] [review] tag: emit the GtkTextTag::changed signal This is a new signal. It permits to redraw the view when a GtkSourceTag property changes. This relies on this GTK+ bug to be fixed: https://bugzilla.gnome.org/show_bug.cgi?id=755416
Created attachment 312030 [details] [review] tag: call gtk_text_tag_changed() in set_property() It's a new function, it relies on this GTK+ bug to be fixed: https://bugzilla.gnome.org/show_bug.cgi?id=755416
Kai, do you think the boolean property is ok? Another way is to have flags to tell which types of spaces to draw, which can be more convenient than applying the tag space-by-space. The boolean property is simpler to implement, and I don't know if it's very useful to have flags. Worst case the API is just a bit less convenient to use, but it doesn't change its powerfulness.
For my use case, the Boolean property is definitely enough. Allowing flags sounds nice, but also sounds like something that wouldn't actually get used, and as you say, it doesn't actually change what you can do with the API.
Ok, thanks for your quick reply. Let's keep the boolean then.
The commits are pushed to the master branch, and will be available for the 3.20 version. Some API changes can still be done during the development cycle.