[PATCH 0/2] gitk: Fix error display when Tcl is too old

[PATCH 0/2] gitk: Fix error display when Tcl is too old

[Bernt Hansen]

The following patch series cleans up error message reporting when your
version of Tcl is too old.

The old code checked the Tcl version first and then tried to report
the error with show_error.  show_error uses msgcat for translation but
msgcat is not yet initialized when we are checking the Tcl version
requirement.

The first patch moves the initialization of msgcat before the check
for the Tcl version.  This version will fail is msgcat is not
available.

The second patch handles the case where the msgcat package is not
available by providing a default mc procedure than just returns the
argument text unchanged (essentially bypassing message text
translation).

This lets us continue to use show_error as-is.

Bernt Hansen (2):
  gitk: Initialize msgcat before first use
  gitk: Provide a default mc function if msgcat is not available

 gitk |   46 ++++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 20 deletions(-)

[PATCH 1/2] gitk: Initialize msgcat before first use

[Bernt Hansen]

The error text generated when your version of Tcl is too old is
translated with msgcat (mc) before msgcat is initialized.  This
causes Tcl to abort with:

    Error in startup script: invalid command name "mc"

We now initialize msgcat first before we check the Tcl version.  Msgcat
is available since Tcl 8.1.

Signed-off-by: Bernt Hansen <bernt@norang.ca>
---
 gitk |   41 +++++++++++++++++++++--------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/gitk b/gitk
index a0214b7..d4cd566 100755
--- a/gitk
+++ b/gitk
@@ -11004,7 +11004,27 @@ proc get_path_encoding {path} {
     return $tcl_enc
 }
 
-# First check that Tcl/Tk is recent enough
+# First setup up mc for translating text
+## For msgcat loading, first locate the installation location.
+if { [info exists ::env(GITK_MSGSDIR)] } {
+    ## Msgsdir was manually set in the environment.
+    set gitk_msgsdir $::env(GITK_MSGSDIR)
+} else {
+    ## Let's guess the prefix from argv0.
+    set gitk_prefix [file dirname [file dirname [file normalize $argv0]]]
+    set gitk_libdir [file join $gitk_prefix share gitk lib]
+    set gitk_msgsdir [file join $gitk_libdir msgs]
+    unset gitk_prefix
+}
+
+## Internationalization (i18n) through msgcat and gettext. See
+## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
+package require msgcat
+namespace import ::msgcat::mc
+## And eventually load the actual message catalog
+::msgcat::mcload $gitk_msgsdir
+
+# Check that Tcl/Tk is recent enough
 if {[catch {package require Tk 8.4} err]} {
     show_error {} . [mc "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
 		     Gitk requires at least Tcl/Tk 8.4."]
@@ -11096,25 +11116,6 @@ if {[tk windowingsystem] eq "aqua"} {
     set ctxbut <Button-3>
 }
 
-## For msgcat loading, first locate the installation location.
-if { [info exists ::env(GITK_MSGSDIR)] } {
-    ## Msgsdir was manually set in the environment.
-    set gitk_msgsdir $::env(GITK_MSGSDIR)
-} else {
-    ## Let's guess the prefix from argv0.
-    set gitk_prefix [file dirname [file dirname [file normalize $argv0]]]
-    set gitk_libdir [file join $gitk_prefix share gitk lib]
-    set gitk_msgsdir [file join $gitk_libdir msgs]
-    unset gitk_prefix
-}
-
-## Internationalization (i18n) through msgcat and gettext. See
-## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
-package require msgcat
-namespace import ::msgcat::mc
-## And eventually load the actual message catalog
-::msgcat::mcload $gitk_msgsdir
-
 catch {source ~/.gitk}
 
 font create optionfont -family sans-serif -size -12
-- 
1.6.5.1.69.g36942

[PATCH 2/2] gitk: Provide a default mc function if msgcat is not available

[Bernt Hansen]

Msgcat is available since Tcl 8.1.  For really old versions of Tcl
provide a default mc that just returns the text untranslated.  This
allows the Tcl version check to return the error in a window instead
of making Tcl abort when attempting to load the msgcat package.

Signed-off-by: Bernt Hansen <bernt@norang.ca>
---
I'm not sure if we care about Tcl versions older than 8.1 but this at
least shows the error in the window with the [OK] button.

 gitk |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/gitk b/gitk
index d4cd566..bff891d 100755
--- a/gitk
+++ b/gitk
@@ -11019,10 +11019,15 @@ if { [info exists ::env(GITK_MSGSDIR)] } {
 
 ## Internationalization (i18n) through msgcat and gettext. See
 ## http://www.gnu.org/software/gettext/manual/html_node/Tcl.html
-package require msgcat
-namespace import ::msgcat::mc
-## And eventually load the actual message catalog
-::msgcat::mcload $gitk_msgsdir
+if {[catch {package require msgcat}]} {
+    proc mc {arg} {
+	return $arg
+    }
+} else {
+    namespace import ::msgcat::mc
+    ## And eventually load the actual message catalog
+    ::msgcat::mcload $gitk_msgsdir
+}
 
 # Check that Tcl/Tk is recent enough
 if {[catch {package require Tk 8.4} err]} {
-- 
1.6.5.1.69.g36942

Re: [PATCH 2/2] gitk: Provide a default mc function if msgcat is not available

[Junio C Hamano]

Bernt Hansen <bernt@norang.ca> writes:

> Msgcat is available since Tcl 8.1.  For really old versions of Tcl
> provide a default mc that just returns the text untranslated.  This
> allows the Tcl version check to return the error in a window instead
> of making Tcl abort when attempting to load the msgcat package.
>
> Signed-off-by: Bernt Hansen <bernt@norang.ca>
> ---
> I'm not sure if we care about Tcl versions older than 8.1 but this at
> least shows the error in the window with the [OK] button.

Both patches sound very sensible.

Re: [PATCH 1/2] gitk: Initialize msgcat before first use

[Pat Thoyts]

Bernt Hansen <bernt@norang.ca> writes:

>The error text generated when your version of Tcl is too old is
>translated with msgcat (mc) before msgcat is initialized.  This
>causes Tcl to abort with:
>
>    Error in startup script: invalid command name "mc"
>
>We now initialize msgcat first before we check the Tcl version.  Msgcat
>is available since Tcl 8.1.
>

This doesn't quite work. [file normalize] was introduced with Tcl 8.4
and when I test this by starting it using Tcl 8.3 I get an error:
 "bad option "normalize": must be atime, attributes, channels..."
from line 11014. It is probably sufficient to just drop the [file
normalize] here. On Windows $argv0 is fully qualified and 
[file dirname] works ok on it. By removing the [file normalize] I get
the expected error dialog when testing with 8.3.

However, on Windows we actually get a better looking result by not
catching the [package require Tcl 8.4] and just letting Tk bring up a
standard message box with the version conflict error message.

Well, actually if show_error just used tk_messageBox it would look
better on Windows.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Re: [PATCH 1/2] gitk: Initialize msgcat before first use

[Bernt Hansen]

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

> Bernt Hansen <bernt@norang.ca> writes:
>
>>The error text generated when your version of Tcl is too old is
>>translated with msgcat (mc) before msgcat is initialized.  This
>>causes Tcl to abort with:
>>
>>    Error in startup script: invalid command name "mc"
>>
>>We now initialize msgcat first before we check the Tcl version.  Msgcat
>>is available since Tcl 8.1.
>>
>
> This doesn't quite work. [file normalize] was introduced with Tcl 8.4
> and when I test this by starting it using Tcl 8.3 I get an error:
>  "bad option "normalize": must be atime, attributes, channels..."
> from line 11014. It is probably sufficient to just drop the [file
> normalize] here. On Windows $argv0 is fully qualified and 
> [file dirname] works ok on it. By removing the [file normalize] I get
> the expected error dialog when testing with 8.3.
> However, on Windows we actually get a better looking result by not
> catching the [package require Tcl 8.4] and just letting Tk bring up a
> standard message box with the version conflict error message.
>
> Well, actually if show_error just used tk_messageBox it would look
> better on Windows.

You're right.  Thanks for catching this.  When I tested this code I
bumped the version number temporarily to 8.5 instead of downgrading TK
to 8.3.

The problem I was trying to fix was show_error using mc internally
before it was initialized.  Maybe it would be better to give show_error
an optional parameter that controls calling mc - so that the call for
the version check can just bypass the mc translation of the text and OK
buttons.  With this approach the code I moved around can just stay where
it is and all of the existing calls to show_error will use the default
parameter setting which invokes mc.

Would that be a better approach?

-Bernt

[PATCH] Skip translation of wrong Tcl version text

[Bernt Hansen]

We check the required Tcl version number before we setup msgcat for
language translation.  If the Tcl version is too old just display the
untranslated error text.

The caller of show_error can now pass an alternative function for mc.
The Tcl list function turns the transalation into a no-op.

This fixes the
    Error in startup script: invalid command name "mc"
when attempting to start gitk with Tcl 8.3.

Signed-off-by: Bernt Hansen <bernt@norang.ca>
---
I tested this patch with both Tcl 8.3 and 8.4.

This is an alternative to the previous 2 patches I sent attempting
to initialize msgcat before first use.  This patch is much simpler
but does not attempt to translate the wrong version message text.

This patch fixes the version number error message by displaying it
untranslated since msgcat is not initialized yet.  The current
initialization code for msgcat uses normalize which is only available as
of Tcl 8.4 so moving the code up front didn't work in Tcl 8.3.

 gitk |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gitk b/gitk
index a0214b7..d1f32a9 100755
--- a/gitk
+++ b/gitk
@@ -1787,10 +1787,10 @@ proc make_transient {window origin} {
     }
 }
 
-proc show_error {w top msg} {
+proc show_error {w top msg {mc mc}} {
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
-    button $w.ok -text [mc OK] -command "destroy $top"
+    button $w.ok -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"
@@ -11006,8 +11006,8 @@ proc get_path_encoding {path} {
 
 # First check that Tcl/Tk is recent enough
 if {[catch {package require Tk 8.4} err]} {
-    show_error {} . [mc "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
-		     Gitk requires at least Tcl/Tk 8.4."]
+    show_error {} . "Sorry, gitk cannot run with this version of Tcl/Tk.\n\
+		     Gitk requires at least Tcl/Tk 8.4." list
     exit 1
 }
 
-- 
1.6.5.2.141.gc8a58

Re: [PATCH] Skip translation of wrong Tcl version text

[Paul Mackerras]

Bernt Hansen writes:

> We check the required Tcl version number before we setup msgcat for
> language translation.  If the Tcl version is too old just display the
> untranslated error text.
> 
> The caller of show_error can now pass an alternative function for mc.
> The Tcl list function turns the transalation into a no-op.
> 
> This fixes the
>     Error in startup script: invalid command name "mc"
> when attempting to start gitk with Tcl 8.3.
> 
> Signed-off-by: Bernt Hansen <bernt@norang.ca>

Thanks, applied.

Paul.