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 742148 - gtksourceview should support grid lines in style schemes
gtksourceview should support grid lines in style schemes
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
git master
Other Linux
: Normal enhancement
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2014-12-31 11:59 UTC by Christian Hergert
Modified: 2015-01-25 09:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Provide a way to draw background patterns (15.12 KB, patch)
2015-01-21 13:24 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
Provide a way to draw background patterns (14.58 KB, patch)
2015-01-21 13:30 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
Provide a way to draw background patterns (14.57 KB, patch)
2015-01-21 13:34 UTC, Ignacio Casal Quinteiro (nacho)
committed Details | Review

Description Christian Hergert 2014-12-31 11:59:17 UTC
I think the grid lines we added to Builder have turned out really nice. It might be time to consider making grid lines an option in the style schemes.

I don't know if we need to have horizontal vs vertical an option. I also don't know if we want to support different color lines every Nth line (so you see blocks of blocks).

Obviously this was a hack. But hey, it worked.

https://git.gnome.org/browse/gnome-builder/tree/src/editor/gb-source-view.c#n1923
Comment 1 Paolo Borelli 2014-12-31 12:29:38 UTC
Do you envision this being part of the style scheme only or a property on the view? I mean, do you expect:

<background-pattern-color>pink</background-pattern-color>

and then

gtk_source_view_set_background_pattern(view, GRID)


or do you expect the style schemes to have

<background-pattern>grid</background-pattern>
<background-pattern-color>pink</background-pattern-color>
Comment 2 Christian Hergert 2014-12-31 12:42:16 UTC
That's a good question.

Personally, I was thinking of adding properties to the <style> elements.

<style name="text" background-line-vertical="pink" background-line-horizontal="purple"/>

But that probably isn't enough if we want to support multiple levels of highlight.

It's not very visible, but mozilla has a good example of what i mean with two highlight colors for the grid. https://developer.mozilla.org/en-US/docs/Web/CSS/filter

And afranke mentioned that he wanted to be able to turn it on for any style scheme. See https://bugzilla.gnome.org/show_bug.cgi?id=741739 for that bug.

So I'm not totally sure yet what the right answer is.
Comment 3 Paolo Borelli 2014-12-31 14:09:05 UTC
My guess would be to just have a property on the view, with an expandable enum for the patterns we want

Then have the colors in the scheme, maybe even with a reasonable default (e.g. darker(bgcolor) if the scheme does not specify one
Comment 4 Christian Hergert 2015-01-04 07:50:21 UTC
(In reply to comment #3)
> My guess would be to just have a property on the view, with an expandable enum
> for the patterns we want
> 
> Then have the colors in the scheme, maybe even with a reasonable default (e.g.
> darker(bgcolor) if the scheme does not specify one

Agreed, I'd be happy to see it implemented as such.
Comment 5 Ignacio Casal Quinteiro (nacho) 2015-01-21 13:24:21 UTC
Created attachment 295096 [details] [review]
Provide a way to draw background patterns

A new property background-pattern is added to support
different kind of patterns. For now only Grid is supported.
Also a new background-pattern style has been added so it
can be used by the style schemes, if this is not provided
a default style is used by picking the background color
and making it a bit darker.
Comment 6 Ignacio Casal Quinteiro (nacho) 2015-01-21 13:30:52 UTC
Created attachment 295097 [details] [review]
Provide a way to draw background patterns

A new property background-pattern is added to support
different kind of patterns. For now only Grid is supported.
Also a new background-pattern style has been added so it
can be used by the style schemes.
Comment 7 Ignacio Casal Quinteiro (nacho) 2015-01-21 13:34:09 UTC
Created attachment 295098 [details] [review]
Provide a way to draw background patterns

A new property background-pattern is added to support
different kind of patterns. For now only Grid is supported.
Also a new background-pattern style has been added so it
can be used by the style schemes.
Comment 8 Paolo Borelli 2015-01-21 13:42:34 UTC
Review of attachment 295098 [details] [review]:

Implementation looks good to me (one mini nit-pick below)


I am not a native english speaker, but when I looked for the translation of "foglio a quadretti" in english, it suggests "checked", so I wonder if we should call the enum value _CHECKED instead of _GRID... Christian?


I am marking as commit now, but maybe let's hold off until we can get Christian or another native speaker to comment on the term

::: gtksourceview/gtksourceview.c
@@ +2727,3 @@
+
+			/*
+			pango_layout_set_text (layout, "X", 1);

indentation here is off by one
Comment 9 Christian Hergert 2015-01-21 22:14:40 UTC
(In reply to comment #8)
> I am marking as commit now, but maybe let's hold off until we can get Christian
> or another native speaker to comment on the term

"Checked" to me would be interpreted like a checkered flag at the end of a F1 race. Or the rendered background when viewing a transparent image.

My vote would be to keep grid.

One thing we might want to consider in the future, is if we should outline ever Nth line slightly darker. This was more work than I felt it was worth in Builder, but might be something to consider now that this is shared burden :)

Patchwise, everything looks good, happy to see this merge upstream!
Comment 10 Ignacio Casal Quinteiro (nacho) 2015-01-22 07:22:20 UTC
Attachment 295098 [details] pushed as eabb2d0 - Provide a way to draw background patterns
Comment 11 Christian Hergert 2015-01-24 10:33:53 UTC
It looks like the grid lines are drawn in the wrong order. They are drawn above the highlight line instead of below.
Comment 12 Paolo Borelli 2015-01-25 09:10:49 UTC
I wondered if that was intentional yesterday... I guess it is not :)

Fixed on master.

By the way, I added colors for the grid to the color schemes shipped with gtksourceview. Let me know if any of them does not look ok.