threads / discuss / 35533

gitk's back button broken (Refactor per-line part of getblobdiffline and its support)

Subject: gitk's back button broken (Refactor per-line part of getblobdiffline and its support)

## tl;dr

3 messages between Dec 16, 2013 and Dec 19, 2013.

replies: 2people: 2as markdown or json

Johannes Sixt· Dec 16, 2013, 13:56 UTC · lore

To reproduce, start gitk in any repository, click a commit, then the "back" button (left-pointing arrow button) or type Alt+Cursor-Left. The error I get is this:

can't use non-numeric string as operand of "!"
can't use non-numeric string as operand of "!"
    while executing
"if {!$ateof} {
	set nlines [expr {[winfo height $ctext]
			  / [font metrics textfont -linespace]}]
	if {[$ctext compare "$target_scrollpos + $nlines ..."
    (procedure "maybe_scroll_ctext" line 5)
    invoked from within
"maybe_scroll_ctext ateof"
    (procedure "blobdiffmaybeseehere" line 6)
    invoked from within
"blobdiffmaybeseehere [eof $bdf]"
    (procedure "getblobdiffline" line 15)
    invoked from within
"getblobdiffline filef5ec60 5de460a2cfdd4acbadd41355a5b34dc69e93b89e"
    ("eval" body line 1)
    invoked from within
"eval $script"
    (procedure "dorunq" line 11)
    invoked from within
"dorunq"
    ("after" script)

After dismissing the error, gitk is still responsive, but does not show diffs anymore.

The error bisects to 5de460a2c (gitk: Refactor per-line part of getblobdiffline and its support) which is already merged downstream into git.

Could you please have a look?

-- Hannes

Johannes Sixt· Dec 19, 2013, 07:14 UTC · re: Johannes Sixt · lore

[PATCH] gitk: Fix typo in proc blobdiffmaybeseehere

From: Johannes Sixt <j6t@kdbg.org>

The recent 5de460a2 (Refactor per-line part of getblobdiffline and its support) introduced blobdiffmaybeseehere, and accidentally forgot the '$' to access the parameter as a TCL variable. This resulted in a failing "Back" button with the error

can't use non-numeric string as operand of "!"
    while executing
"if {!$ateof} {
	set nlines [expr {[winfo height $ctext]
			  / [font metrics textfont -linespace]}]
	if {[$ctext compare "$target_scrollpos + $nlines ..."
    (procedure "maybe_scroll_ctext" line 5)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 12/16/2013 14:56, schrieb Johannes Sixt:

Show 5 quoted lines

> To reproduce, start gitk in any repository, click a commit, then the
> "back" button (left-pointing arrow button) or type Alt+Cursor-Left. The
> error I get is this:
> 
> can't use non-numeric string as operand of "!"

It turns out to be just a simple typo.

-- Hannes

 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 33c3a6c..1f14796 100755
--- a/gitk
+++ b/gitk
@@ -7922,7 +7922,7 @@ proc blobdiffmaybeseehere {ateof} {
     if {$diffseehere >= 0} {
 	mark_ctext_line [lindex [split $diffseehere .] 0]
     }
-    maybe_scroll_ctext ateof
+    maybe_scroll_ctext $ateof
 }
 
 proc getblobdiffline {bdf ids} {

-- 
1.8.5.1.1587.g3845a3d

Thomas Rast· Dec 19, 2013, 23:16 UTC · re: Johannes Sixt · lore

Re: [PATCH] gitk: Fix typo in proc blobdiffmaybeseehere

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 18 quoted lines

> can't use non-numeric string as operand of "!"
>     while executing
> "if {!$ateof} {
> 	set nlines [expr {[winfo height $ctext]
> 			  / [font metrics textfont -linespace]}]
> 	if {[$ctext compare "$target_scrollpos + $nlines ..."
>     (procedure "maybe_scroll_ctext" line 5)
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Am 12/16/2013 14:56, schrieb Johannes Sixt:
>> To reproduce, start gitk in any repository, click a commit, then the
>> "back" button (left-pointing arrow button) or type Alt+Cursor-Left. The
>> error I get is this:
>> 
>> can't use non-numeric string as operand of "!"
>
> It turns out to be just a simple typo.

[...]

> -    maybe_scroll_ctext ateof
> +    maybe_scroll_ctext $ateof
>  }

Ew.  Sorry about that!

It's very polite of you to call it a typo, but I think it's safe to blame it on my lack of practice in Tcl :-)

-- 
Thomas Rast
tr@thomasrast.ch