GNOME Bugzilla – Bug 85821
Preference for changing cursor color
Last modified: 2016-02-13 20:21:47 UTC
Hi Havoc, heres another patch for GNOME2.0.x Originally I just wanted to add code for a custom color popup (see the screenshot). Since I initially thought I would have to store some of the popup values in gconf I came to the terminals quite unmaintainable property code, saw it fixed it to use a struct of bitfields, put common task into preprocessor macros. Since the are alot of prefs the patch became quite big: cvs diff got serious problems to diff out the new terminal_profile_update. Well... Later it pointed out that it would be stupid to store some values of the popup in gconf, those values are aproximated online by reading the colorpots now. Since I'm too lazy to separate both patches (popup and prefscleanup) right now I'm sending you the combined patch, tell me if I shall separate them. Hope that I and "cvs diff" didn't broke something.
Created attachment 9305 [details] [review] the monster patch
Created attachment 9306 [details] the custom-palette-popup
I like the general direction of the settings cleanup, thanks for doing that - it looks like a lot of work! I'm not sure I understand what happens if I click the "initialize custom palette" popup though - it seems like it may be a useful idea to avoid doing the custom palette by hand, though. There are various small changes I'd make to the cleanup; e.g. I have a thing where I only use while loops, I put a space around : in "foo:1", and I'd really prefer it if macros allowed a semicolon at the end so that Emacs indentation worked. etc. I would like to handle these two patches separately; presumably the cleanup comes first, then the color popup. My feeling is that I'd like to think through the color UI a bit better; one suggestion that's been made is that we have one idea of schema/palette (one option menu) that includes both background/foreground and the full palette, since the full palette really assumes either a light or a dark foreground/background. Both patches need to be on the 2.1.x branch instead of the 2.0.1 branch, and the 2.1.x branch doesn't exist yet; I'll probably create it after we've done the UI review bugs for 2.0.1. I forget the bug number for that but it's one of the newest gnome-terminal bugs if you run a query.
bug 85655, FWIW, is the ui-review bug.
This would be nice to mop up a bit for 2.2
I've broken out Mathias's monster patch into just the allowing more settings part since I also want to add another setting. The diff is against GNOME_TERMINAL_2_0_1. One thing I changed with the original patch is that in a couple of places emit_changed was being called with a pointer to a stack local variable as the signal data which didn't seem like a good idea to me, so I made the setting mask class private. Someone with more gtk know how should determine if this is the right way to do it. The other patch allows the user to set the text cursor color. It also requires a patch to vte.
Created attachment 11746 [details] [review] more than 32 settings
Created attachment 11747 [details] [review] colorized text cursor
Created attachment 11748 [details] [review] vte support for colorized text cursor
What's the rationale for overriding the text cursor color? The current behavior is to draw the cell(s) in with the foreground and background colors reversed, which seems more correct to me.
The rationale is that it's easier to find with the eye especially if you've turned off cursor blink and have inverse text on your terminal. Lets say you're in an editor marking a block of text (which is shown by inverse), when the cursor is at the edge of the block, you can't tell which is the cursor and which is the block. You are right that the foreground color should be reversed. This helps when the cursor color is the same as the text color. The *fore = *back line should be taken out of the vte patch to get the same (desirable) behaviour as xterm with a color cursor.
I applied the patch for allowing more prefs, before it bitrotted. Thanks for that. The cursor color thing can't go in for 2.2 as I missed the freeze; for reviewing this, it would be helpful to have a comprehensive "rework the palette stuff" proposal including the changes Nalin wanted to make, and a plan for what that tab of the profile editor dialog ends up looking like.
I don't use vte so I'm unfamiliar with its features... but I hope the inverse video/custom text cursors are just an option :) To be accessible the cursor needs to be drawn in the gtk-specified cursor colour by default.
Is this still valid?
Probably. It has become a multi headed RFE... The “more than 32 settings” patch has been applied and Nalin added cursor color support to vte, so the patch for that is not longer necessary. That leaves the patch to add the cursor color and the one to initialize the custom palette...
Comment on attachment 11748 [details] [review] vte support for colorized text cursor per mariano's comment.
Updated summary to reflect what this has actually become.
Wonder why this had the string keyword...
Created attachment 81253 [details] [review] Follow the "cursor-color" style property With this patch on vte, VteTerminal follows the cursor-color style property set on the style, so that style "test" { VteTerminal::cursor-color = "red" } class "GtkWidget" style "test" in your ~/.gtkrc-2.0 file will get you red cursors. According to Calum, this should be the default, right? If we want to provide user-colorizable cursors, we need UI like the one proposed in a patch in bug 69776, but vte would also need to provide a vte_terminal_set_follow_the_gtk_cursor_color to toggle this...
*** Bug 69776 has been marked as a duplicate of this bug. ***
Bug 69776 has a nice screen shot for a proposed UI for this on g-t's side.
Can't we just make it follow Gtk+'s cursor color unconditionally?
The patch is way too naive. We have to deal with vte_terminal_set_color_cursor somehow. While we are at it, vte_terminal_set_color_highlight should use the style's GTK_STATE_SELECTED to do highlighting, and take the background color from the style, too. And deal with their vte_terminal_set_* functions, too.
*** Bug 148378 has been marked as a duplicate of this bug. ***
Any progress on this? Right now one might use multiple profiles all having different color schemes for background and foreground color as a way of differentiating between different hosts. Very useful, especially when every session is associated with a modified Gnome Terminal icon which indicates the color scheme in question. That way, I can, subconsciously select the right session when doing alt-tab. Adding the ability to also customize the cursor color for different profiles would make it possible to keep track of even more different hosts, and yes, I would find it very useful. (Compiled and started latest stable gnome-terminal, version 2.26.3, but there were no support for custom cursor color there. To my disappointment, the ability to specify a custom icon for each profile had ALSO disappeared... Hm... but that last issue is not the topic of this particular bug so I'll leave it for now)
we still don't have a way to alter the cursor color here, and it should be part of the profile preferences in gnome-terminal. there should also be some intelligence to not layer a bright cursor color over a bright character (i.e. inverting the text color if it's necessary, to increase contrast for readability). color selection should be possible via a color picker popup. - armin
(In reply to Armin Jarmusch from comment #26) > there should also be some > intelligence to not layer a bright cursor color over a bright character > (i.e. inverting the text color if it's necessary, to increase contrast for > readability). Bug #695011 covers that problem, so it should be marked as a dependency of this one. It also proposes adding support for the cursor foreground colour to g-t's UI so the two bugs could also be merged, but as g-t still doesn't have a cursor (background) colour option I think they're slightly separate issues.
*** Bug 695011 has been marked as a duplicate of this bug. ***
I've implemented this and raised a pull request on github <https://github.com/GNOME/gnome-terminal/pull/3>.
Thanks! Could you please attach the patch series here (either with git-bz, or just as output from git format-patch) so I can git-am it? (We don't use github pull requests.)
Created attachment 320897 [details] git-format-patch output for implementation of cursor colour profile options I wasn't sure how to package these files for attachment, I hope a tarball is OK.
Thanks for the patches! I've split the non-UI changes out and committed that for 3-20.
I had to change the .UI file anyway since it was totally broken with newer gtk+, so I've implemented the UI in a different way.