Good point.  Can't get my gedit to respect it though.  Any idea?

Works here using 

style "cursorstyle" { GtkWidget::cursor-aspect-ratio = 0.5 } class "*" style "cursorstyle"

(that probably not the most elegant / simplest method to get this, but it works).

I have:

style "tony" {
	GtkWidget::cursor-aspect-ratio = 0.1
	GtkWidget::cursor-color = "#c00040"
	#bg[SELECTED] = "#6081ab"
	bg[SELECTED] = "#859fb8"
}
class "*" style "tony"

I did have class "GtkWidget" but I just changed it to "*" after seeing the last message to see if that would help, but it doesn't.

It works for me with most other widgets, including GtkSourceView (gedit), so it's strange that it doesn't work there for Behdad. Firefox/Iceweasel has always ignored these cursor settings for me BTW.

(Just FYI, that should be fixed in gecko 1.9.1 / ff 3.5, see  https://bugzilla.mozilla.org/show_bug.cgi?id=118308 .)

Works now.  Thanks.

Looking into fixing in vte.

I presume comment 5 just means you got it working in gedit? It still doesn't work for me in Firefox. I think the fix above resulted in cursor-color working, but not cursor-aspect-ratio so I've opened a new bug <https://bugzilla.mozilla.org/show_bug.cgi?id=532717>.

Yes, I wasn't talking about firefox.

Created attachment 158184 [details] [review]
Use the style's cursor-aspect-ratio when drawing the cursor

For the ibeam cursor, use the aspect ratio; for the underline, its
inverse (that's the only thing that makes sense here).

Bug #586950.

Looks good.  Any reason to use float instead of double?

Well, the style property is float not double, so I just kept it that way. I don't care either way though, so if you want to store the and calculate as doubles, I'll change that.

Fair enough.  Feel free to commit then.

Pushed to master.

Humm, for underline cursor I feel like using the same width as the I-beam is closer to my expectation as a user.  What do you think?

I guess you mean same *height* as the width of of the ibeam cursor (because clearly the underline cursor's width should be the full cell) ? I don't know, that seems odd to me.

I'd like it to be the same thickness rather than the same aspect. My aim in changing the style is to make it slightly thicker (2px instead of 1px at usual font sizes), and I think the underline cursor would generally be slightly shorter than the beam, so it might be prone to reverting to 1px thick.

However, I don't use the underline cursor, so perhaps my opinion on it shouldn't be taken with any weight :-).

I agree with Tony.  I expect same thickness too.

(In reply to comment #2)
> style "cursorstyle" { GtkWidget::cursor-aspect-ratio = 0.5 } class "*" style
> "cursorstyle"

To makes sure it's jotted down somewhere, so I can find it again, for gtk 3 this can be achieved (for just vte) with:

$ cat .config/gtk-3.0/gtk.css 
VteTerminal {
    -GtkWidget-cursor-aspect-ratio: 0.1;
}

(In reply to comment #16)
> I agree with Tony.  I expect same thickness too.

Ditto.

The underline's width is smaller than the ibeam's height (when using a font that's higher than it's wide, which is true for many fonts). So the current code makes the surface of the underline quite a bit smaller than the ibeam's surface, for any aspect ratio. And a reasonable cursor aspect ratio makes the underline hard to spot (for my eyes).

And indeed, using a trivial patch to make the underline's height equal the ibeam's width, gives an underline that look better (to me). That is, once the local cursor aspect ratio pushes the ibeam's width to two pixels (and thus the underline's height to two too).

Created attachment 264611 [details] [review]
make underline's height equal the ibeam's width

(In reply to comment #17) 
> And indeed, using a trivial patch to make the underline's height equal the
> ibeam's width, gives an underline that look better (to me). That is, once the
> local cursor aspect ratio pushes the ibeam's width to two pixels (and thus the
> underline's height to two too).

I just built vte3 0.34.9 with the trivial patch I've carried since 0.30.1. I've attached it for anyone wanting to see whether it really improves things.

+1 for applying Paul's patch, please :)

+1 :-)

Fixed in 0-36, keeping open for vte-next.

Fixed on master.