threads / patch / 21345

patch, 2 partsgitk: Fix error display when Tcl is too old

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

## tl;dr

8 messages between Oct 24, 2009 and Nov 3, 2009. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Bernt Hansen· Oct 24, 2009, 20:20 UTC · lore

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(-)
Bernt Hansen· Oct 24, 2009, 20:20 UTC · re: Bernt Hansen · lore

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

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(-)
Show changes to gitk +21 −20
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
Pat Thoyts· Oct 31, 2009, 21:34 UTC · re: Bernt Hansen · lore

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

Bernt Hansen <bernt@norang.ca> writes:
Show 9 quoted lines
>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
Bernt Hansen· Nov 1, 2009, 13:09 UTC · re: Pat Thoyts · lore

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

Pat Thoyts <patthoyts@users.sourceforge.net> writes:
Show 25 quoted lines
> 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
Bernt Hansen· Nov 2, 2009, 12:41 UTC · re: Bernt Hansen · lore

[PATCH] Skip translation of wrong Tcl version text

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(-)
Show changes to gitk +4 −4
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
Paul Mackerras· Nov 3, 2009, 11:31 UTC · re: Bernt Hansen · lore

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

Bernt Hansen writes:
Show 12 quoted lines
> 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.
Bernt Hansen· Oct 24, 2009, 20:20 UTC · re: Bernt Hansen · lore

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

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(-)
Show changes to gitk +9 −4
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
Junio C Hamano· Oct 24, 2009, 22:08 UTC · re: Bernt Hansen · lore

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

Bernt Hansen <bernt@norang.ca> writes:
Show 9 quoted lines
> 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.

← back to recent threads