GNOME Bugzilla – Bug 697048
GtkTextView: small code improvements
Last modified: 2013-04-01 19:16:51 UTC
As I'm reading the code, I prefer to improve things at the same time.
Created attachment 240308 [details] [review] Doc: move the GtkTextAttributes struct It's more logical to have the GtkTextAttributes and GtkTextAppearance structs together. And it creates a separation between gtk_text_tag and gtk_text_attributes functions.
Created attachment 240309 [details] [review] Doc: remove dead link to gtk_widget_override_text() The function doesn't exist.
Created attachment 240310 [details] [review] gtktexttypes: remove inline_byte_begins_utf8_char() The function was used only in gtk_text_byte_begins_utf8_char().
Created attachment 240311 [details] [review] gtktextattributes: remove outdated comment GTK_WRAPMODE_CHAR etc don't exist anymore, it seems that the names have changed. Anyway the comment is not really useful.
Created attachment 240312 [details] [review] gtktextattributes: include the right headers Some function prototypes in gtktexttagprivate.h are implemented in gtktextattributes.c.
Created attachment 240313 [details] [review] gtktextattributes: explain what is "pg_bg" I didn't know what "pg" stands for.
Created attachment 240314 [details] [review] GtkTextView: remove some dead code
Created attachment 240315 [details] [review] GtkTextView: minor code style improvements
Created attachment 240316 [details] [review] GtkTextTagTable: simplify a bit the code
Review of attachment 240308 [details] [review]: OK
Review of attachment 240309 [details] [review]: I believe the intention was to mention gtk_widget_override_color() here instead.
Review of attachment 240310 [details] [review]: OK
Review of attachment 240311 [details] [review]: I'd just update the comment with the correct values.
Review of attachment 240312 [details] [review]: OK
Review of attachment 240313 [details] [review]: Sure
Review of attachment 240314 [details] [review]: OK
Review of attachment 240315 [details] [review]: Honestly this is quite some churn for no real value...shuffling code around also makes it harder to follow the code history e.g. with git bisect. In general I'd rather not take this patch, except for this part below. ::: gtk/gtktextattributes.h @@ +106,3 @@ + * As with most GTK+ structs, the fields in this struct should only + * be read, never modified directly. + */ This (moving the comment above the relevant struct) is OK
Review of attachment 240316 [details] [review]: Nice cleanup.
Thanks for the review! I've adapted and pushed the commits.