git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] gitk: Replaced "green" with "#00FF00".
@ 2012-12-27 12:59 Peter Hofmann
  2012-12-27 17:27 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Hofmann @ 2012-12-27 12:59 UTC (permalink / raw)
  To: git; +Cc: gitster

The definition of "green" has changed in Tk 8.6:

- http://wiki.tcl.tk/21276
- http://www.tcl.tk/cgi-bin/tct/tip/403

gitk looks pretty awkward with Tk 8.6. "green" is simply too dark now
because it has changed from "#00FF00" to "#008000".

One could also use "lime" instead of "#00FF00" but that would break
compatibility with older versions of Tk.

Signed-off-by: Peter Hofmann <git-dev@uninformativ.de>
---
 gitk-git/gitk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..d7e922b 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2209,7 +2209,7 @@ proc makewindow {} {
 	set h [expr {[font metrics uifont -linespace] + 2}]
 	set progresscanv .tf.bar.progress
 	canvas $progresscanv -relief sunken -height $h -borderwidth 2
-	set progressitem [$progresscanv create rect -1 0 0 $h -fill green]
+	set progressitem [$progresscanv create rect -1 0 0 $h -fill "#00FF00"]
 	set fprogitem [$progresscanv create rect -1 0 0 $h -fill yellow]
 	set rprogitem [$progresscanv create rect -1 0 0 $h -fill red]
     }
@@ -2342,7 +2342,7 @@ proc makewindow {} {
     $ctext tag conf dresult -fore [lindex $diffcolors 1]
     $ctext tag conf m0 -fore red
     $ctext tag conf m1 -fore blue
-    $ctext tag conf m2 -fore green
+    $ctext tag conf m2 -fore "#00FF00"
     $ctext tag conf m3 -fore purple
     $ctext tag conf m4 -fore brown
     $ctext tag conf m5 -fore "#009090"
@@ -3226,7 +3226,7 @@ set rectmask {
        0x00, 0x00, 0xfc, 0x0f, 0xfc, 0x0f, 0xfc, 0x0f,
        0xfc, 0x0f, 0xfc, 0x0f, 0xfc, 0x0f, 0xfc, 0x0f, 0x00, 0x00};
 }
-image create bitmap reficon-H -background black -foreground green \
+image create bitmap reficon-H -background black -foreground "#00FF00" \
     -data $rectdata -maskdata $rectmask
 image create bitmap reficon-o -background black -foreground "#ddddff" \
     -data $rectdata -maskdata $rectmask
@@ -5909,7 +5909,7 @@ proc drawcmittext {id row col} {
     if {$id eq $nullid} {
 	set ofill red
     } elseif {$id eq $nullid2} {
-	set ofill green
+	set ofill "#00FF00"
     } elseif {$id eq $mainheadid} {
 	set ofill yellow
     } else {
@@ -6377,7 +6377,7 @@ proc drawtags {id x xt y1} {
 	} else {
 	    # draw a head or other ref
 	    if {[incr nheads -1] >= 0} {
-		set col green
+		set col "#00FF00"
 		if {$tag eq $mainhead} {
 		    set font mainfontbold
 		}
@@ -11617,7 +11617,7 @@ if {[tk windowingsystem] eq "aqua"} {
     set extdifftool "meld"
 }
 
-set colors {green red blue magenta darkgrey brown orange}
+set colors {"#00FF00" red blue magenta darkgrey brown orange}
 if {[tk windowingsystem] eq "win32"} {
     set uicolor SystemButtonFace
     set bgcolor SystemWindow
-- 
1.8.0.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] gitk: Replaced "green" with "#00FF00".
  2012-12-27 12:59 [PATCH] gitk: Replaced "green" with "#00FF00" Peter Hofmann
@ 2012-12-27 17:27 ` Junio C Hamano
  2012-12-31 23:21   ` Paul Mackerras
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2012-12-27 17:27 UTC (permalink / raw)
  To: Peter Hofmann; +Cc: git, Paul Mackerras

Peter Hofmann <git-dev@uninformativ.de> writes:

> Subject: Re: [PATCH] gitk: Replaced "green" with "#00FF00".

That should be

	Subject: [PATCH] gitk: replace "green" with "#00FF00"

around here.  Instead of reporting what you did in the past tense,
you give an order to somebody to do something to make the codebase
into more desirable shape, without the final full-stop.

> The definition of "green" has changed in Tk 8.6:
>
> - http://wiki.tcl.tk/21276
> - http://www.tcl.tk/cgi-bin/tct/tip/403

Concise reference to the background information is very much
appreciated.  It would have been even nicer if you wrote one line
summary of it, e.g. "This change was to make Tk applications more in
line with Web standard colors."

> gitk looks pretty awkward with Tk 8.6. "green" is simply too dark now
> because it has changed from "#00FF00" to "#008000".

Your observation "awkward" is somewhat subjective and I am hesitant
to recommend this change without a better justification.  Given the
reasoning behind the change Tcl/Tk people made, I wouldn't be
surprised if people coming from webapp world view the "green" color
rendered by updated Tcl/Tk more natural.

Besides, if we are declaring with this patch that we will stick to
X11 colors and will not adopt W3C colors, the patch shouldn't update
only "green", but set all the other colors in stone, no?  "purple",
for example, is also different between X11 and W3C.

> One could also use "lime" instead of "#00FF00" but that would break
> compatibility with older versions of Tk.

A better solution might be to make these colors customizable.

> Signed-off-by: Peter Hofmann <git-dev@uninformativ.de>
> ---
>  gitk-git/gitk | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Please make gitk patches against 

	url = git://ozlabs.org/~paulus/gitk.git

which is my upstream maintained by Paul Mackerras <paulus@samba.org>
(cc'ed) and keep him in the loop.

A patch against Paul's tree would have these lines

> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index d93bd99..d7e922b 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk

without "/gitk-git".

Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gitk: Replaced "green" with "#00FF00".
  2012-12-27 17:27 ` Junio C Hamano
@ 2012-12-31 23:21   ` Paul Mackerras
  0 siblings, 0 replies; 3+ messages in thread
From: Paul Mackerras @ 2012-12-31 23:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Peter Hofmann, git

On Thu, Dec 27, 2012 at 09:27:37AM -0800, Junio C Hamano wrote:
> Peter Hofmann <git-dev@uninformativ.de> writes:
> 
> > Subject: Re: [PATCH] gitk: Replaced "green" with "#00FF00".
> 
> > gitk looks pretty awkward with Tk 8.6. "green" is simply too dark now
> > because it has changed from "#00FF00" to "#008000".
> 
> Your observation "awkward" is somewhat subjective and I am hesitant
> to recommend this change without a better justification.  Given the
> reasoning behind the change Tcl/Tk people made, I wouldn't be
> surprised if people coming from webapp world view the "green" color
> rendered by updated Tcl/Tk more natural.

Given that "green" is used as the background color in some places,
e.g. for the boxes containing the names of heads, and that the general
scheme is dark foreground on light background, I agree that #008000 is
too dark in those places.

> Besides, if we are declaring with this patch that we will stick to
> X11 colors and will not adopt W3C colors, the patch shouldn't update
> only "green", but set all the other colors in stone, no?  "purple",
> for example, is also different between X11 and W3C.

Purple is only used for octopus merges.  I'd like to think of a better
way to use color in representing octopus merges if possible.

> > One could also use "lime" instead of "#00FF00" but that would break
> > compatibility with older versions of Tk.
> 
> A better solution might be to make these colors customizable.

Indeed.  Some people prefer to have all their windows to have light
foregrounds on dark backgrounds, so they would also benefit from
having more of the colors customizable.

Paul.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-12-31 23:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-27 12:59 [PATCH] gitk: Replaced "green" with "#00FF00" Peter Hofmann
2012-12-27 17:27 ` Junio C Hamano
2012-12-31 23:21   ` Paul Mackerras

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).