<paolo> hp: how can I change the cursor color in a GtkTextView? (I'm
trying to fix bug #79585)
<hp> paolo: I don't think we have that feature, I could be wrong
<paolo> hp: hmmm... then I think I will not fix this buf :-( 
cursor-color is a style property, can this info help me?
<hp> paolo: well it means people can set a cursor color in their gtkrc
anyway

I think at the moment we have no way to fix this bug.

luis: should we fill a bug report against GtkTextView?

Since it is not fixable -> normal

Moving to GTK until GTK exports a way to set the cursor color, 
or magically picks the right cursor color, or something.
Maybe a tag property for cursor color?

I don't think using tags will be a good solution, at least to solve
gedit bug.
I think we should have a function (for example:
gtk_widget_modify_cursor) like gtk_widget_modify_text and family

*** Bug 84548 has been marked as a duplicate of this bug. ***

This has probably already been considered and rejected, but...

One common behaviour on other platforms seems to be to have the cursor
invert whatever is in the area occupied by it, rather than picking
just one colour for the whole thing. Of course, that can make the
cursor hard to spot in some other cases, but surely it would do the
right thing more often than now?

What's annoying is that there seems to be a "cursor-color" property in
GtkWidget but I can't figure out how to change it, or even if I'm
supposed to be able to. I'm still rather new to GTK+ :-)

The "cursor-color" property there is intended to be set by the theme,
for the default color.

*** Bug 97698 has been marked as a duplicate of this bug. ***

*** Bug 100577 has been marked as a duplicate of this bug. ***

I've fixed this for the accessibility themes (see #100330), guess
someone just needs to fix it in the default theme now if it's still
happening :)

*** Bug 102114 has been marked as a duplicate of this bug. ***

*** Bug 105840 has been marked as a duplicate of this bug. ***

*** Bug 108492 has been marked as a duplicate of this bug. ***

*** Bug 118953 has been marked as a duplicate of this bug. ***

You *can* manage to change the cursor color by setting a 
name on the widget and using gtk_rc_parse_string(). Not
nice at all though.

Created attachment 88907 [details] [review]
patch

Does someone have an idea why cursor uses style->black instead of style->text? If it uses style->text, then modify_text() will correctly change the cursor color along with text color.

Hmm, but hardcoding the state here looks a bit wrong. And if you use the actual state of the widget, then the cache ought to keep track of the state as well.

And what about the secondary cursor ? 

(In reply to comment #17)
> Hmm, but hardcoding the state here looks a bit wrong.

It does look wrong to me too, but it's what actually is done - cursor color is the text color, and the text color is text[GTK_STATE_NORMAL]. Should I make cursor_gc's be created for different states, and fetch appropriate state's gc in draw_insertion_cursor() (I don't think cursor is drawn for GtkTextView and GtkEntry when those are not GTK_STATE_NORMAL or not in focus)?

> And if you use the actual
> state of the widget, then the cache ought to keep track of the state as well.

I thought it's taken care of in gtk_style_real_init_from_rc, cursor gc's are destroyed if text[GTK_STATE_NORMAL] changed. I don't know style mechanics at all though, so I might have guessed wrong.

> And what about the secondary cursor ? 

I wish someone could explain what this secondary cursor is. Perhaps take cursor color and tansform it somehow (whatever it's called, when you make grey of black)?

> It does look wrong to me too, but it's what actually is done - cursor color is
> the text color, and the text color is text[GTK_STATE_NORMAL]. Should I make
> cursor_gc's be created for different states, and fetch appropriate state's gc
> in draw_insertion_cursor() (I don't think cursor is drawn for GtkTextView and
> GtkEntry when those are not GTK_STATE_NORMAL or not in focus)?

Hmm, I guess you are right, but at least it is worth a short comment.

I wish someone could explain what this secondary cursor is. Perhaps take cursor
color and tansform it somehow (whatever it's called, when you make grey of
black)?

 
> I wish someone could explain what this secondary cursor is.

If you want to see it in action, go to the entry example in testgtk, first entry,
and move the cursor to the very end. A secondary cursor is shown when insertion of 
LTR and RTL text would happen at different places.

Created attachment 88927 [details] [review]
patch

Added comments, and made secondary cursor color be average between text and base colors. Don't ask about magical numbers.

Created attachment 88928 [details] [review]
patch

Oops, unrelated change slipped into the patch.

I just want to point out that the patch will not work for themes that set the "cursor-color" or "secondary-cursor-color" style properties.
That said I like the patch as it chooses saner default colors. (A problem we are running into in themes when the user chooses a dark color scheme.)

Why won't it work? It does work.

Ehm, I am saying that it will not work *if* the theme sets the style properties. Because then the color returned by make_cursor_gc will be from the style property and *not* the text color.


For example put the following style into your gtkrc and remove the hack that gedit has to set the cursor-color.

style "blub"
{
        GtkWidget::cursor-color = "#0000ff"
}

class "GtkWidget" style "blub"

The cursor-color is blue, and stays blue, whatever you do to the gedit preferences. Your patch does not make any difference.

If I understand it correctly, then it works as expected. The patch isn't mean to break themes; if a theme sets cursor color then this set color is used, as it should. Am I missing something?

Well, you still end up with a fixed cursor color. This color may not be the desired one with a (user) modified background color. And I thought that was the whole point of this bug.

Yes, got it now. So it seems we do need api to change cursor color (right now it's problem in GtkSourceView which simply may not use gtkrc trick, since it may not assign widget names and it may not invent its own themes on top of user's).

Matthias, would it be okay to make per-widget style matching, like it's done for widget names? Perhaps something like: when widget's own style is to be modified, create GtkStyle object, attach it with some g_object_set_data(widget, "gtk-widget-personal-style"), and make it participate in theming business?

This is what the gtk_widget_modify_* functions do. A GtkRcStyle is attached to the widget, and merged in last, overriding the themes rc styles.

What you said could be done with the current system if if it was possible to not only modify the colors of the rc style, but also set style properties. For this to work one would basically need to make insert_rc_property (from gtkrc.c) public API to make that work.

(In reply to comment #28)
> This is what the gtk_widget_modify_* functions do. A GtkRcStyle is attached to
> the widget, and merged in last, overriding the themes rc styles.

That's right, and GtkRcStyle won't do style properties.

> What you said could be done with the current system if if it was possible to
> not only modify the colors of the rc style, but also set style properties. For
> this to work one would basically need to make insert_rc_property (from gtkrc.c)
> public API to make that work.

And the question is if it would be okay to do it (it's totally good that it's easier than I thought).

I think I am fine with changing get_insertion_cursor_gc() to use 
text[NORMAL] and something between text and base for the secondary cursor.
You can simply use text_aa for that though, considering

      style->text_aa[i].red = (style->text[i].red + style->base[i].red) / 2;
      style->text_aa[i].green = (style->text[i].green + style->base[i].green) / 2;
      style->text_aa[i].blue = (style->text[i].blue + style->base[i].blue) / 2;

I also think that extending the gtk_widget_modify_... api to allow
setting style properties might be an acceptable way to reduce the perceived
need in apps to muck around with gtk_rc_parse_string()

Committed this colors thing: use text[GTK_STATE_NORMAL] for primary cursor, and text_aa[GTK_STATE_NORMAL] for secondary cursor.

Created attachment 89164 [details] [review]
gtk_rc_style_set_rc_property() (GtkTextView API to change cursor color)

<mclasen> muntyan: we could let them jump through one more hoop by confining it to gtk_rc_style_set_property()

Here are gtk_rc_style_set_rc_property() and gtk_rc_style_unset_rc_property(), rather ugly.

Created attachment 89202 [details] [review]
gtk_widget_style_set* (GtkTextView API to change cursor color)

Here's gtk_widget_style_set* family and gtk_widget_style_unset_property().

Created attachment 89314 [details] [review]
patch

Or perhaps this one, with gtk_widget_modify_cursor().

*** Bug 312139 has been marked as a duplicate of this bug. ***

Lets take the modify_cursor approach first and worry about setting other style properties later.

- The docs are missing a "Since: 2.12"

- Please use gdk_color_to_string instead of printf

- Please add the new function to docs/reference/gtk/gtk-sections.txt


Other than that, the patch looks good, please commit with these changes

Done, with said changes.

2007-06-06  Yevgen Muntyan  <muntyan@tamu.edu>

        * gtk/gtkwidget.c:
        * gtk/gtkwidget.h: new method, gtk_widget_modify_cursor() (#89314).

        * gtkrc.c:
        * gtkrc.h: new functions _gtk_rc_style_set_rc_property() and
        _gtk_rc_style_unset_rc_property().

        * gtk/gtk.symbols: added gtk_widget_modify_cursor.

        * tests/testtext.c (do_cursor_visible_changed):
        * tests/testgtk.c (create_styles): test it.

It's unfortunate that the textview's naming of its cursor clashes with the other more generic concept of a cursor (mouse pointer) used in gdk. Adding API on GtkWidget with cursor in the name sounds more like it has to do with the mouse cursor than a GtkTextView/Entry specific thing, in my opinion. I don't have a better suggestion though.

Perhaps make it gtk_widget_modify_cursor_color()? I personally like it better, but I made it consistent with other gtk_widget_modify_stuff() names.

Hmm. Yes, there is a slight clash in naming, but the use of "cursor" as 
"text cursor/caret" is pretty widespread:

gtk-cursor-blink
cursor-color
gtk-split-cursor
gtk_text_view_move_cursor

I think the consistency with the rest of the gtk_widget_modify_ family
of functions outweigths this concern.