* Re: [PATCH/gdbtk] Make color of changed vars customizable v2
@ 2012-05-25 10:42 Roland Schwingel
0 siblings, 0 replies; 3+ messages in thread
From: Roland Schwingel @ 2012-05-25 10:42 UTC (permalink / raw)
To: insight
Hi ...
Thanks for reviewing Keith!
I applied all your wishes regarding formatting.
> > @@ -54,7 +54,7 @@ itcl::body SrcPref::_build_win {} {
> > set w [$f.colors get_frame]
> >
> > set color [pref get gdb/src/PC_TAG]
> > - label $w.pcl -text {PC}
> > + label $w.pcl -text {PC/Var change}
> > button $w.pcb -text { } -activebackground $color -bg $color
> > -command [code $this _pick $color $w.pcb PC_TAG]
>
> How about if we just use the more generic term, "Highlight"
> instead of "PC/Var change"?
Changed the label to "Highlight". You are right this is the better term.
Committed the patch now.
Roland
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH/gdbtk] Make color of changed vars customizable v2
2012-04-11 12:27 Roland Schwingel
@ 2012-05-24 21:08 ` Keith Seitz
0 siblings, 0 replies; 3+ messages in thread
From: Keith Seitz @ 2012-05-24 21:08 UTC (permalink / raw)
To: Roland Schwingel; +Cc: insight
On 04/11/2012 05:26 AM, Roland Schwingel wrote:
> 2012-04-11 Roland Schwingel <roland.schwingel@onevision.com>
>
> * library/locals.tcl: Updated copyright.
> (reconfig): New method for getting prefs updates.
> * library/prefs.tcl: Updated copyright.
> (pref_read): Moved comment. Take color for changed
> fields from gdb/src/PC_TAG prefs value.
> (pref_set_defaults): Change default of
> PC_TAG color to a somewhat darker green.
> (pref_set_colors): Call pref_load_default in central place.
> (pref_set_option_db): Remove hard wired old color for
> changed values.
> * library/regwin.itb: Update copyright.
> (reconfig): Update highlight color of register table.
> * library/srcpref.itb: Update copyright.
> (_build_win): Change text of PC color chooser.
> (_apply): Update global changed fields color from PC_TAG color.
> * library/vartree.itb (update_var): use changed field color from
> global colors list.
> (_init_data): Remove local copy of changed vars color.
>
> Any comments? Is this ok?
This looks pretty good. Some minor nits below.
> diff -ruNp gdbtk_orig/library/prefs.tcl gdbtk/library/prefs.tcl
> --- gdbtk_orig/library/prefs.tcl 2008-03-04 00:25:03.000000000 +0100
> +++ gdbtk/library/prefs.tcl 2012-04-11 13:59:37.809259100 +0200
> @@ -1,5 +1,5 @@
> # Local preferences functions for Insight.
> -# Copyright (C) 1997, 1998, 1999, 2002, 2003, 2004, 2008 Red Hat
> +# Copyright (C) 1997-2012 Red Hat, Inc.
> #
> # This program is free software; you can redistribute it and/or modify it
> # under the terms of the GNU General Public License (GPL) as published by
> @@ -146,14 +146,18 @@ proc pref_read {} {
> if {[pref get gdb/use_color_schemes] != "1"} {
> pref_set_colors $home
> } else {
> - global Colors
> # These colors are the same for all schemes
> + global Colors
> set Colors(textfg) black
> set Colors(fg) black
> set Colors(sbg) \#4c59a5
> set Colors(sfg) white
> set_bg_colors
> }
> +
^^^ This line appears to have extraneous whitespace on it. Please
double-check.
> + # set color for changes
> + global Colors
> + set Colors(change) [pref get gdb/src/PC_TAG]
> }
Please capitalize/attempt to use full sentences for comments. "Set color
for changes" . Period at the end? Maybe. I don't really care too much
for imperatives like this.
>
> # ------------------------------------------------------------------
> @@ -333,12 +337,12 @@ proc pref_set_defaults {} {
> pref define gdb/console/wrap 0
> pref define gdb/console/prompt_fg DarkGreen
> pref define gdb/console/error_fg red
> - pref define gdb/console/log_fg green
> + pref define gdb/console/log_fg \#00b300
There appear to be extra whitespaces at the end of this line, too.
Please double-check.
> diff -ruNp gdbtk_orig/library/srcpref.itb gdbtk/library/srcpref.itb
> --- gdbtk_orig/library/srcpref.itb 2008-08-03 00:08:32.000000000 +0200
> +++ gdbtk/library/srcpref.itb 2012-04-11 14:00:33.563757100 +0200
> @@ -1,5 +1,5 @@
> # Source preferences dialog for Insight.
> -# Copyright (C) 1998, 1999, 2002, 2003, 2008 Red Hat
> +# Copyright (C) 1998-2012 Red Hat, Inc.
> #
> # This program is free software; you can redistribute it and/or modify it
> # under the terms of the GNU General Public License (GPL) as published by
> @@ -54,7 +54,7 @@ itcl::body SrcPref::_build_win {} {
> set w [$f.colors get_frame]
>
> set color [pref get gdb/src/PC_TAG]
> - label $w.pcl -text {PC}
> + label $w.pcl -text {PC/Var change}
> button $w.pcb -text { } -activebackground $color -bg $color \
> -command [code $this _pick $color $w.pcb PC_TAG]
How about if we just use the more generic term, "Highlight" instead of
"PC/Var change"? I think the original choice to use "PC" is poor, since
it really means *current* PC (as opposed to PC of the non-current
frame). We highlight all kinds of things, register/variable changes, the
current PC, etc.
> @@ -212,6 +212,10 @@ itcl::body SrcPref::_apply {} {
> #debug "$var = $_new($var)"
> pref set $var $_new($var)
> }
> +
> + # update color for changed values
> + global Colors
> + set Colors(change) [pref get gdb/src/PC_TAG]
Extra whitespace on first of these four lines. Please double-check.
Similar comment as before regarding comments.
> }
> if {$_new_disassembly_flavor != ""} {
> gdb_cmd "set disassembly-flavor $_new_disassembly_flavor"
> diff -ruNp gdbtk_orig/library/vartree.itb gdbtk/library/vartree.itb
> --- gdbtk_orig/library/vartree.itb 2010-04-29 23:00:52.000000000 +0200
> +++ gdbtk/library/vartree.itb 2012-04-11 14:01:25.371448100 +0200
> @@ -135,7 +135,7 @@ itcl::body VarTree::update_var {var ena
> if {[catch {$var value} value]} {
> set color $colors(error)
> } elseif {[$c itemcget $val -text] != $value} {
> - set color $colors(change)
> + set color $::Colors(change)
> } else {
> set color $colors(value)
> }
> @@ -404,7 +404,6 @@ itcl::body VarTree::_init_data {} {
> set colors(type) "red"
> set colors(error) "red"
> set colors(value) "black"
> - set colors(change) $::Colors(change)
> set colors(disabled) "gray50"
> set colors(line) "gray50"
>
>
Okay with those changes.
Keith
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH/gdbtk] Make color of changed vars customizable v2
@ 2012-04-11 12:27 Roland Schwingel
2012-05-24 21:08 ` Keith Seitz
0 siblings, 1 reply; 3+ messages in thread
From: Roland Schwingel @ 2012-04-11 12:27 UTC (permalink / raw)
To: insight
[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]
Hi...
This patch superseeds my previous patch from here:
http://sourceware.org/ml/insight/2012-q1/msg00031.html
As the old patch is yet not reviewed I reworked it a bit.
The new patch additionally takes also care about the register's window
changed fields color and it does some minor simplifications.
Changelog:
2012-04-11 Roland Schwingel <roland.schwingel@onevision.com>
* library/locals.tcl: Updated copyright.
(reconfig): New method for getting prefs updates.
* library/prefs.tcl: Updated copyright.
(pref_read): Moved comment. Take color for changed
fields from gdb/src/PC_TAG prefs value.
(pref_set_defaults): Change default of
PC_TAG color to a somewhat darker green.
(pref_set_colors): Call pref_load_default in central place.
(pref_set_option_db): Remove hard wired old color for
changed values.
* library/regwin.itb: Update copyright.
(reconfig): Update highlight color of register table.
* library/srcpref.itb: Update copyright.
(_build_win): Change text of PC color chooser.
(_apply): Update global changed fields color from PC_TAG color.
* library/vartree.itb (update_var): use changed field color from
global colors list.
(_init_data): Remove local copy of changed vars color.
Any comments? Is this ok?
Roland
[-- Attachment #2: custom_color_changed_vars2.patch --]
[-- Type: text/plain, Size: 6494 bytes --]
diff -ruNp gdbtk_orig/library/locals.tcl gdbtk/library/locals.tcl
--- gdbtk_orig/library/locals.tcl 2006-12-01 04:34:39.000000000 +0100
+++ gdbtk/library/locals.tcl 2012-04-11 13:58:01.853490600 +0200
@@ -1,5 +1,5 @@
# Local Variable Window for Insight.
-# Copyright (C) 2002, 2003, 2006 Red Hat
+# Copyright (C) 2002-2012 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License (GPL) as published by
@@ -45,6 +45,14 @@ itcl::class LocalsWin {
cursor watch
}
+ # ------------------------------------------------------------------
+ # PUBLIC METHOD: reconfig - used when preferences change
+ # ------------------------------------------------------------------
+ method reconfig {} {
+ debug
+ $tree update
+ }
+
# Re-enable the UI
method idle {event} {
debug
diff -ruNp gdbtk_orig/library/prefs.tcl gdbtk/library/prefs.tcl
--- gdbtk_orig/library/prefs.tcl 2008-03-04 00:25:03.000000000 +0100
+++ gdbtk/library/prefs.tcl 2012-04-11 13:59:37.809259100 +0200
@@ -1,5 +1,5 @@
# Local preferences functions for Insight.
-# Copyright (C) 1997, 1998, 1999, 2002, 2003, 2004, 2008 Red Hat
+# Copyright (C) 1997-2012 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License (GPL) as published by
@@ -146,14 +146,18 @@ proc pref_read {} {
if {[pref get gdb/use_color_schemes] != "1"} {
pref_set_colors $home
} else {
- global Colors
# These colors are the same for all schemes
+ global Colors
set Colors(textfg) black
set Colors(fg) black
set Colors(sbg) \#4c59a5
set Colors(sfg) white
set_bg_colors
}
+
+ # set color for changes
+ global Colors
+ set Colors(change) [pref get gdb/src/PC_TAG]
}
# ------------------------------------------------------------------
@@ -333,12 +337,12 @@ proc pref_set_defaults {} {
pref define gdb/console/wrap 0
pref define gdb/console/prompt_fg DarkGreen
pref define gdb/console/error_fg red
- pref define gdb/console/log_fg green
+ pref define gdb/console/log_fg \#00b300
pref define gdb/console/target_fg blue
pref define gdb/console/font global/fixed
# Source window defaults
- pref define gdb/src/PC_TAG green
+ pref define gdb/src/PC_TAG \#00b300
pref define gdb/src/STACK_TAG gold
pref define gdb/src/BROWSE_TAG \#9595e2
pref define gdb/src/handlebg red
@@ -490,6 +494,7 @@ proc pref_set_colors {home} {
# then Insight won't be able to display sources and highlight things properly.
# Therefore we will not change the textfg and textbg.
+ pref_load_default
switch [pref get gdb/compat] {
"Windows" {
@@ -508,7 +513,6 @@ proc pref_set_colors {home} {
"KDE" {
debug "loading OS colors for KDE"
- pref_load_default
# try loading "~/.gtkrc-kde"
if {[pref_load_gnome $home [list .gtkrc-kde]]} {
debug "loaded gnome file"
@@ -550,13 +554,11 @@ proc pref_set_colors {home} {
}
"GNOME" {
- pref_load_default
pref_load_gnome $home
pref_set_option_db 0
}
"default" {
- pref_load_default
pref_set_option_db 1
}
}
@@ -719,11 +721,6 @@ proc load_gnome_file {fd} {
proc pref_set_option_db {makebg} {
global Colors
- # The color of text that indicates changed items
- # We standardize on one color here so that changed
- # items don't blend into any OS color scheme
- set Colors(change) "green"
-
option add *background $Colors(bg)
option add *buttonBackground $Colors(bg)
option add *Text*background $Colors(textbg)
diff -ruNp gdbtk_orig/library/regwin.itb gdbtk/library/regwin.itb
--- gdbtk_orig/library/regwin.itb 2007-06-27 22:50:50.000000000 +0200
+++ gdbtk/library/regwin.itb 2012-04-11 13:59:49.758880100 +0200
@@ -1,5 +1,5 @@
# Register display window for Insight.
-# Copyright (C) 1998, 1999, 2001, 2002, 2003, 2004, 2007 Red Hat, Inc.
+# Copyright (C) 1998-2012 Red Hat, Inc.
#
# Written by Keith Seitz (keiths@redhat.com)
# and Martin Hunt (hunt@redhat.com)
@@ -420,6 +420,7 @@ itcl::body RegWin::_size_column {col dow
itcl::body RegWin::reconfig {} {
$itk_component(table) tag configure normal \
-state disabled -bg $::Colors(textbg) -fg $::Colors(textfg)
+ $itk_component(table) tag configure highlight -bg $::Colors(change) -fg black
}
diff -ruNp gdbtk_orig/library/srcpref.itb gdbtk/library/srcpref.itb
--- gdbtk_orig/library/srcpref.itb 2008-08-03 00:08:32.000000000 +0200
+++ gdbtk/library/srcpref.itb 2012-04-11 14:00:33.563757100 +0200
@@ -1,5 +1,5 @@
# Source preferences dialog for Insight.
-# Copyright (C) 1998, 1999, 2002, 2003, 2008 Red Hat
+# Copyright (C) 1998-2012 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License (GPL) as published by
@@ -54,7 +54,7 @@ itcl::body SrcPref::_build_win {} {
set w [$f.colors get_frame]
set color [pref get gdb/src/PC_TAG]
- label $w.pcl -text {PC}
+ label $w.pcl -text {PC/Var change}
button $w.pcb -text { } -activebackground $color -bg $color \
-command [code $this _pick $color $w.pcb PC_TAG]
@@ -212,6 +212,10 @@ itcl::body SrcPref::_apply {} {
#debug "$var = $_new($var)"
pref set $var $_new($var)
}
+
+ # update color for changed values
+ global Colors
+ set Colors(change) [pref get gdb/src/PC_TAG]
}
if {$_new_disassembly_flavor != ""} {
gdb_cmd "set disassembly-flavor $_new_disassembly_flavor"
diff -ruNp gdbtk_orig/library/vartree.itb gdbtk/library/vartree.itb
--- gdbtk_orig/library/vartree.itb 2010-04-29 23:00:52.000000000 +0200
+++ gdbtk/library/vartree.itb 2012-04-11 14:01:25.371448100 +0200
@@ -135,7 +135,7 @@ itcl::body VarTree::update_var {var ena
if {[catch {$var value} value]} {
set color $colors(error)
} elseif {[$c itemcget $val -text] != $value} {
- set color $colors(change)
+ set color $::Colors(change)
} else {
set color $colors(value)
}
@@ -404,7 +404,6 @@ itcl::body VarTree::_init_data {} {
set colors(type) "red"
set colors(error) "red"
set colors(value) "black"
- set colors(change) $::Colors(change)
set colors(disabled) "gray50"
set colors(line) "gray50"
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-05-25 10:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-25 10:42 [PATCH/gdbtk] Make color of changed vars customizable v2 Roland Schwingel
-- strict thread matches above, loose matches on Subject: below --
2012-04-11 12:27 Roland Schwingel
2012-05-24 21:08 ` Keith Seitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).