GNOME Bugzilla – Bug 767216
style-schemes: use the style scheme for diagnostician underlining
Last modified: 2016-06-06 19:34:08 UTC
Let me know if anyone can think of a cleaner way to write the fall-back code.
Created attachment 329088 [details] [review] style-schemes: use the style scheme for diagnostician underlining Builder underlines segments of code in different colors depending on the condition reported by libclang (deprecated, error, note, or warning). As of v3.17.2, GtkSourceView supports the underline-color attribute in style schemes. So Builder now makes use of that, and falls back on hard coded values in case our custom style scheme isn't installed.
Review of attachment 329088 [details] [review]: Logic/implementation looks good. Just some style nits for our codebase. Thanks! ::: libide/ide-buffer.c @@ +1032,3 @@ if (style_scheme != NULL) { + // These are used as a fall-back if our style scheme isn't installed. We are trying to be consistent with comment styling and use /* */ everywhere. @@ +1038,3 @@ + gdk_rgba_parse (&warning_rgba, WARNING_COLOR); + + if (!ide_source_style_scheme_apply_style(style_scheme, Space before open paren. @@ +1039,3 @@ + + if (!ide_source_style_scheme_apply_style(style_scheme, + TAG_DEPRECATED, Align multi-line parameter lists @@ +1044,3 @@ + "underline", PANGO_UNDERLINE_ERROR, + "underline-rgba", &deprecated_rgba, + NULL); Extra newline between unrelated conditional blocks @@ +1108,3 @@ IdeBuffer *self = (IdeBuffer *)object; IdeBufferPrivate *priv = ide_buffer_get_instance_private (self); + GtkTextTagTable *tag_table = gtk_text_buffer_get_tag_table (GTK_TEXT_BUFFER (self)); We try to only do assignment/casts or get_instance_private() calls in our declarations (which is not actually a call, its a pointer+offset so safe from accidental dereference). So move the assignments after the declaration block. @@ +1110,3 @@ + GtkTextTagTable *tag_table = gtk_text_buffer_get_tag_table (GTK_TEXT_BUFFER (self)); + GtkSourceStyleScheme *style_scheme = gtk_source_buffer_get_style_scheme (GTK_SOURCE_BUFFER (self)); + GtkTextTag *deprecated_tag; Maybe g_autoptr(GtkTextTag) deprecated_tag = NULL; and drop the g_object_unref()s at the end of the function? @@ +1121,3 @@ G_OBJECT_CLASS (ide_buffer_parent_class)->constructed (object); + // These are used as a fall-back if our style scheme isn't installed. /* */ @@ +1143,3 @@ + "underline", PANGO_UNDERLINE_ERROR, + "underline-rgba", &deprecated_rgba, + NULL); Extra newline between unrelated blocks. @@ +1160,3 @@ + NULL); + + gtk_text_tag_table_add(tag_table, deprecated_tag); Space before paren ::: libide/ide-source-style-scheme.c @@ -92,2 +98,4 @@ - if (underline_set && underline) - g_object_set (tag, "underline", PANGO_UNDERLINE_SINGLE, NULL); + if (underline_set) + g_object_set (tag, "underline", pango_underline, NULL); + + if (underline_color_set && underline_color) { We use gtk/gnu style curly braces (which can take a little time to get used to) and spaces before open parens. For pointers, we are trying to follow GNOME guidelines and check explicitly for NULL. For g_object_get() and g_object_set() we try to do a single key/value pair per-line to make it trivial to check for trailing NULL. So altogether, that would look like: if (underline_color_set && underline_color != NULL) { gdk_rgba_parse (&underline_rgba, underline_color); g_object_set (tag, "underline-rgba", &underline_rgba, NULL); }
Created attachment 329092 [details] [review] style-schemes: use the style scheme for diagnostics underlining Builder underlines segments of code in different colors depending on the condition reported by libclang (deprecated, error, note, or warning). As of v3.17.2, GtkSourceView supports the underline-color attribute in style schemes. So Builder now makes use of that, and falls back on hard-coded values in case our custom style scheme isn't installed.
Review of attachment 329092 [details] [review]: LGTM, Thanks!
Thanks for finding this old fixme and fixing it! Attachment 329092 [details] pushed as 97607b9 - style-schemes: use the style scheme for diagnostics underlining