threads / discuss / 30099

gitk: Failure of new tabbed preferences dialog

Subject: gitk: Failure of new tabbed preferences dialog

## tl;dr

8 messages between Mar 29, 2012 and Apr 3, 2012.

replies: 7people: 4as markdown or json

Ramsay Jones· Mar 29, 2012, 17:31 UTC · lore
Hi Pat,

I just tried the new gitk (master @fae9d76) and, since I don't have themed widgets, the preferences menu item uses the emulated tabbed dialog code, which fails like so:

    can't read "col": no such variable
    can't read "col": no such variable
        (reading value of variable to increment)
        invoked from within
    "incr col"
        (procedure "doprefs" line 36)
        invoked from within
    "doprefs"
        (menu invoke)
I tried the following (maybe *too* obvious) fix:
-- >8 --
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 651b740..68cd1a1 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -11012,6 +11012,7 @@ proc doprefs {} {
     lappend pages [prefspage_general $notebook] [mc "General"]
     lappend pages [prefspage_colors $notebook] [mc "Colors"]
     lappend pages [prefspage_fonts $notebook] [mc "Fonts"]
+    set col 0
     foreach {page title} $pages {
 	if {$use_notebook} {
 	    $notebook add $page -text $title
-- 8< --

which seems to work.

HTH

ATB,
Ramsay Jones
Pat Thoyts· Apr 1, 2012, 22:00 UTC · re: Ramsay Jones · lore

Re: gitk: Failure of new tabbed preferences dialog

Show 6 quoted lines
> I just tried the new gitk (master @fae9d76) and, since I don't have
> themed widgets, the preferences menu item uses the emulated tabbed
> dialog code, which fails like so:
>
>     can't read "col": no such variable
>

The issue here is that the incr command has changed between tcl 8.4 and 8.5 and in more recent versions automatically creates the named variable if it did not exist. The fix you suggest is correct. I've also included another that I realised had been applied to msysGit and was not posted upstream.

Pat Thoyts.
Pat Thoyts· Apr 1, 2012, 22:00 UTC · re: Pat Thoyts · lore

[PATCH 1/2] gitk: fix tabbed preferences construction when using tcl 8.4

In 8.5 the incr command creates the target variable if it does not exist but in 8.4 using incr on a non-existing variable raises an error. Ensure we have created our counter variable when creating the tabbed dialog for non-themed preferences.

Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
 gitk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/gitk b/gitk
index 651b740..68cd1a1 100755
--- a/gitk
+++ b/gitk
@@ -11012,6 +11012,7 @@ proc doprefs {} {
     lappend pages [prefspage_general $notebook] [mc "General"]
     lappend pages [prefspage_colors $notebook] [mc "Colors"]
     lappend pages [prefspage_fonts $notebook] [mc "Fonts"]
+    set col 0
     foreach {page title} $pages {
 	if {$use_notebook} {
 	    $notebook add $page -text $title

-- 
1.7.9.msysgit.0
Pat Thoyts· Apr 1, 2012, 22:00 UTC · re: Pat Thoyts · lore

[PATCH 2/2] gitk: fix setting font display with new tabbed dialog layout.

The changes to the dialog window tree broke the preview of the selected font on the button. This corrects that issue.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
 gitk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index 68cd1a1..22270ce 100755
--- a/gitk
+++ b/gitk
@@ -10795,7 +10795,7 @@ proc fontok {} {
     if {$fontparam(slant) eq "italic"} {
 	lappend fontpref($f) "italic"
     }
-    set w $prefstop.$f
+    set w $prefstop.notebook.fonts.$f
     $w conf -text $fontparam(family) -font $fontpref($f)
 
     fontcan

-- 
1.7.9.msysgit.0
Junio C Hamano· Apr 2, 2012, 16:19 UTC · re: Pat Thoyts · lore

Re: gitk: Failure of new tabbed preferences dialog

Pat Thoyts <patthoyts@users.sourceforge.net> writes:
Show 11 quoted lines
>> I just tried the new gitk (master @fae9d76) and, since I don't have
>> themed widgets, the preferences menu item uses the emulated tabbed
>> dialog code, which fails like so:
>>
>>     can't read "col": no such variable
>
> The issue here is that the incr command has changed between tcl 8.4
> and 8.5 and in more recent versions automatically creates the named
> variable if it did not exist. The fix you suggest is correct. I've
> also included another that I realised had been applied to msysGit and
> was not posted upstream.

This seems severe enough that we should put it in the coming release. I can queue them directory to gitk tree (and later ask Paul to pull it from me), and merge the result to expedite the roundtrip.

Junio C Hamano· Apr 2, 2012, 22:03 UTC · re: Junio C Hamano · lore

Re: gitk: Failure of new tabbed preferences dialog

Paul, Ramsay and Pat noticed and fixed two buglets that made the "Font" setting dialog unusable in the version in 1.7.10-rc3. I've queued the fixes below, tested them (before and after applying the patches) locally.

I would like to merge them and tag 1.7.10-rc4 tonight.

In order to keep us in sync, could you please pull back from me? The branch "pt/gitk" corresponds to your repository (i.e. without the rest of git.git, and gitk-git/Makefile as the top-level Makefile).

Thanks.
-- -- --
The following changes since commit b2b76d10696d945bf19318831b64d009d119e051:
  gitk: Teach gitk to respect log.showroot (2012-03-24 16:44:12 +1100)
are available in the git repository at:
  git://github.com/gitster/git.git pt/gitk
for you to fetch changes up to 39ddf99c1db8b230ec7bdefc6ce3c769d1cf4ab6:
  gitk: fix setting font display with new tabbed dialog layout. (2012-04-02 10:21:10 -0700)
----------------------------------------------------------------
Pat Thoyts (2):
      gitk: fix tabbed preferences construction when using tcl 8.4
      gitk: fix setting font display with new tabbed dialog layout.
 gitk |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gitk b/gitk
index 651b740..22270ce 100755
--- a/gitk
+++ b/gitk
@@ -10795,7 +10795,7 @@ proc fontok {} {
     if {$fontparam(slant) eq "italic"} {
 	lappend fontpref($f) "italic"
     }
-    set w $prefstop.$f
+    set w $prefstop.notebook.fonts.$f
     $w conf -text $fontparam(family) -font $fontpref($f)
 
     fontcan
@@ -11012,6 +11012,7 @@ proc doprefs {} {
     lappend pages [prefspage_general $notebook] [mc "General"]
     lappend pages [prefspage_colors $notebook] [mc "Colors"]
     lappend pages [prefspage_fonts $notebook] [mc "Fonts"]
+    set col 0
     foreach {page title} $pages {
 	if {$use_notebook} {
 	    $notebook add $page -text $title
Paul Mackerras· Apr 3, 2012, 12:19 UTC · re: Junio C Hamano · lore

Re: gitk: Failure of new tabbed preferences dialog

On Mon, Apr 02, 2012 at 09:19:00AM -0700, Junio C Hamano wrote:
Show 17 quoted lines
> Pat Thoyts <patthoyts@users.sourceforge.net> writes:
> 
> >> I just tried the new gitk (master @fae9d76) and, since I don't have
> >> themed widgets, the preferences menu item uses the emulated tabbed
> >> dialog code, which fails like so:
> >>
> >>     can't read "col": no such variable
> >
> > The issue here is that the incr command has changed between tcl 8.4
> > and 8.5 and in more recent versions automatically creates the named
> > variable if it did not exist. The fix you suggest is correct. I've
> > also included another that I realised had been applied to msysGit and
> > was not posted upstream.
> 
> This seems severe enough that we should put it in the coming release.
> I can queue them directory to gitk tree (and later ask Paul to pull it
> from me), and merge the result to expedite the roundtrip.

OK; the patches look fine to me. If that's what you're doing, I won't apply them to my tree, to avoid duplication.

Paul.
Junio C Hamano· Apr 3, 2012, 16:53 UTC · re: Paul Mackerras · lore

Re: gitk: Failure of new tabbed preferences dialog

Paul Mackerras <paulus@samba.org> writes:
Show 8 quoted lines
> On Mon, Apr 02, 2012 at 09:19:00AM -0700, Junio C Hamano wrote:
>> 
>> This seems severe enough that we should put it in the coming release.
>> I can queue them directory to gitk tree (and later ask Paul to pull it
>> from me), and merge the result to expedite the roundtrip.
>
> OK; the patches look fine to me.  If that's what you're doing, I won't
> apply them to my tree, to avoid duplication.
Thanks.

← back to recent threads