[PATCH] gitk: Remove mc parameter from proc show_error

[PATCH] gitk: Remove mc parameter from proc show_error

[Alex Henrie]

This is a better fix for 8d849957d81fc0480a52570d66cc3c2a688ecb1b.

All that was required to fix the original issue was to remove the extra
mc call, i.e. change [mc "Sorry, gitk cannot run..."] to simply
"Sorry, gitk cannot run..." Changing the signature of proc show_error
was unnecessary and introduced two new bugs: It made "OK" untranslatable
and "mc" translatable when the opposite should be true.

This new fix makes the string "OK" translatable and the string "mc" not
translatable, while leaving the string "Sorry, gitk cannot run..." not
translatable. It will take effect the next time `make update-po` is run.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 gitk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index 30fcd30..096389f 100755
--- a/gitk
+++ b/gitk
@@ -1894,13 +1894,13 @@ proc make_transient {window origin} {
     }
 }
 
-proc show_error {w top msg {mc mc}} {
+proc show_error {w top msg} {
     global NS
     if {![info exists NS]} {set NS ""}
     if {[wm state $top] eq "withdrawn"} { wm deiconify $top }
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
-    ${NS}::button $w.ok -default active -text [$mc OK] -command "destroy $top"
+    ${NS}::button $w.ok -default active -text [mc OK] -command "destroy $top"
     pack $w.ok -side bottom -fill x
     bind $top <Visibility> "grab $top; focus $top"
     bind $top <Key-Return> "destroy $top"
@@ -12011,7 +12011,7 @@ proc get_path_encoding {path} {
 # First check that Tcl/Tk is recent enough
 if {[catch {package require Tk 8.4} err]} {
     show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
-		     Gitk requires at least Tcl/Tk 8.4." list
+		     Gitk requires at least Tcl/Tk 8.4."
     exit 1
 }
 
-- 
2.3.7

Re: [PATCH] gitk: Remove mc parameter from proc show_error

[Paul Mackerras]

On Fri, May 01, 2015 at 09:13:20PM -0600, Alex Henrie wrote:
> This is a better fix for 8d849957d81fc0480a52570d66cc3c2a688ecb1b.
> 
> All that was required to fix the original issue was to remove the extra
> mc call, i.e. change [mc "Sorry, gitk cannot run..."] to simply
> "Sorry, gitk cannot run..." Changing the signature of proc show_error
> was unnecessary and introduced two new bugs: It made "OK" untranslatable
> and "mc" translatable when the opposite should be true.
> 
> This new fix makes the string "OK" translatable and the string "mc" not
> translatable, while leaving the string "Sorry, gitk cannot run..." not
> translatable. It will take effect the next time `make update-po` is run.

To test this, I changed {package require Tk 8.4} to {package require
Tk 8.7}, in order to deliberately trigger the error.  When I run gitk
with that change (and your patch applied), I get this in the xterm
where I run gitk:

$ ./gitk
Error in startup script: invalid command name "mc"
    while executing
"mc OK"
    (procedure "show_error" line 7)
    invoked from within
"show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n Gitk requires at least Tcl/Tk 8.4.""
    invoked from within
"if {[catch {package require Tk 8.7} err]} {
    show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
                     Gitk requires at least ..."
    (file "./gitk" line 12012)

and no pop-up window.  So this patch still isn't quite right.

Given that old versions of tcl/tk probably don't have [mc], I think
it's inevitable that "OK" will have to be untranslated for that
particular error path.

Paul.

Re: [PATCH] gitk: Remove mc parameter from proc show_error

[Alex Henrie]

2015-05-02 4:05 GMT-06:00 Paul Mackerras <paulus@samba.org>:
> To test this, I changed {package require Tk 8.4} to {package require
> Tk 8.7}, in order to deliberately trigger the error.  When I run gitk
> with that change (and your patch applied), I get this in the xterm
> where I run gitk:
>
> $ ./gitk
> Error in startup script: invalid command name "mc"
>     while executing
> "mc OK"
>     (procedure "show_error" line 7)
>     invoked from within
> "show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n Gitk requires at least Tcl/Tk 8.4.""
>     invoked from within
> "if {[catch {package require Tk 8.7} err]} {
>     show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
>                      Gitk requires at least ..."
>     (file "./gitk" line 12012)
>
> and no pop-up window.  So this patch still isn't quite right.

Good point. We can fix this by importing ::msgcat::mc earlier.

> Given that old versions of tcl/tk probably don't have [mc], I think
> it's inevitable that "OK" will have to be untranslated for that
> particular error path.

msgcat and ::msgcat::mc were added in Tcl 8.1a2, released Febuary 23,
1998. See the "changes" file in
<ftp://ftp.tcl.tk/pub/tcl/tcl8_1/tcl8.1.tar.gz>. Surely you are not
trying to support Tcl 8.0 and earlier, so we should be able to use mc
all we want.

-Alex

[PATCH] gitk: Remove mc parameter from proc show_error

[Alex Henrie]

This partially reverts commit 8d849957d81fc0480a52570d66cc3c2a688ecb1b.

This change makes the string "OK" translatable and the string "mc" not
translatable. It will take effect the next time `make update-po` is run.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 gitk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 30fcd30..7193f6f 100755
--- a/gitk
+++ b/gitk
@@ -1894,13 +1894,13 @@ proc make_transient {window origin} {
     }
 }
 
-proc show_error {w top msg {mc mc}} {
+proc show_error {w top msg} {
     global NS
     if {![info exists NS]} {set NS ""}
     if {[wm state $top] eq "withdrawn"} { wm deiconify $top }
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
-    ${NS}::button $w.ok -default active -text [$mc OK] -command "destroy $top"
+    ${NS}::button $w.ok -default active -text [mc OK] -command "destroy $top"
     pack $w.ok -side bottom -fill x
     bind $top <Visibility> "grab $top; focus $top"
     bind $top <Key-Return> "destroy $top"
-- 
2.3.5

Re: [PATCH] gitk: Remove mc parameter from proc show_error

[Paul Mackerras]

On Thu, Apr 02, 2015 at 03:05:35PM -0600, Alex Henrie wrote:
> This partially reverts commit 8d849957d81fc0480a52570d66cc3c2a688ecb1b.

... and brings back the bug that 8d849957d81f solves, as far as I can
see.  If that's not the case then you need to explain that in the
patch description.

Paul.