GNOME Bugzilla – Bug 762505
snippet background color should come from style scheme
Last modified: 2016-03-04 21:21:37 UTC
Currently the color used when rendering the background for snippet chunks is hard coded to #fcaf3e in ide_source_view_draw_snippet_chunks(). This is not ideal. We should cache the color when the style scheme changes and add a new style to the style scheme (with a fallback to the current color). The style should probably be named "snippet::text".
I'm working on this can you give more information.
1) We need to add new style definitions to data/style-schemes/*.xml Name one snippet::text and set the background="" xml attribute to #fcaf3e Name one snippet::area and set the background="" xml attribute to #204a87 I'll tweak these before release. 2) See libide/ide-source-view.c. Update ide_source_view__buffer_notify_style_scheme_cb() to cache these values from the style scheme as a GdkRGBA in the IdeSourceViewPrivate structure. A good name for the fields might be: GdkRGBA snippet_area_background_rgba; GdkRGBA snippet_text_background_rgba; 3) Modify ide_source_view_draw_snippet_chunks() to use snippet_text_rgba 4) Modify ide_source_view_drag_snippets_background() to use snippet_area_rgba
Created attachment 322944 [details] [review] source-style-scheme: add helper to sync styles to text tags
Created attachment 322945 [details] [review] snippets: use GtkTextTag instead of drawing chunk backgrounds This was always a sort of nasty part of the snippet system, so lets just use GtkTextTags instead. They aren't quite as fancy (we can't do rounded corners yet or anything), but they get the job done and look better with custom backgrounds drawn.
Created attachment 322946 [details] [review] style-schemes: add styling for snippet tab stops
I decided to go a different direction for styling the tab stops and use GtkTextTag to style them instead. We still need to do the sync'ing of the style for the background area though. Attachment 322944 [details] pushed as 0e44a1d - source-style-scheme: add helper to sync styles to text tags Attachment 322945 [details] pushed as 248c59a - snippets: use GtkTextTag instead of drawing chunk backgrounds Attachment 322946 [details] pushed as c1497ed - style-schemes: add styling for snippet tab stops
christian Hergert you already did everything, whats left for me :)
Created attachment 323006 [details] [review] snippets: use background color from style scheme
I have added an attachment for this bug, not sure if it's the appropriate way to tackle the problem. However I noticed another problem in my code: The background area now uses the "snippet::area" style from the scheme file indeed, and the background is updated upon scheme change, but it's one-off, meaning that if I switch to another scheme, modify the "snippet::area" background color of the old scheme and switch back, the changes don't take effect, even restarting the program doesn't help, I have to rebuild the project to see the changes. It seems that the old value is really "cached" on the disk somewhere during the building process.
By "rebuild the project" I mean rebuilding Builder, sorry for the ambiguity.
Review of attachment 323006 [details] [review]: Couple small fixes ::: libide/ide-source-view.c @@ +1013,3 @@ + + g_object_get (snippet_area_style, "background", &background, NULL); + gdk_rgba_parse (&priv->snippet_area_background_rgba, background); It's possible that background is NULL, so we need to fall through to the block below in that case. @@ +1017,3 @@ + else + { + gdk_rgba_parse(&priv->snippet_area_background_rgba, "#204a87"); Add a space before (
(In reply to Akash Talreja from comment #7) > christian Hergert you already did everything, whats left for me :) As I stated, we still need the background fix. We need to get 3.19.92 out the door very soon, and this was one of the glaring issues in dark mode. It needed to be done.
Created attachment 323057 [details] [review] snippets: use background color from style scheme
Now the patch takes care of the case when snippet background is NULL, and comply with the coding style. Still have no idea why it requires a rebuild for the color change to take effect.
Excellent, thanks! Attachment 323057 [details] pushed as b01f5fd - snippets: use background color from style scheme
(As for the color change, if you use `make run`, it will use the style schemes from the source tree, otherwise you have to `make install` to see them in your installation).