git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Fix tree mode of the file list for files containing curly brackets
@ 2008-03-14 21:49 Alex Riesen
  2008-03-14 21:58 ` [PATCH] " Alex Riesen
  2008-04-27 10:57 ` [RESEND] " Alex Riesen
  0 siblings, 2 replies; 5+ messages in thread
From: Alex Riesen @ 2008-03-14 21:49 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Junio C Hamano

As far as I could understand the online documentation the [lindex ...]
thing expects an array, which a string produced by git-ls-tree is not.
So [split ...] it first, to get a real Tcl string-array.

For instance:

    $ git init
    $ date >file
    $ git add . && git commit -m1
    $ git mv file '{a-b}.a.b'
    $ git commit -m2
    $ gitk

Now switch the file list from "Patch" to "Tree":

    list element in braces followed by ".a.b" instead of space
    list element in braces followed by ".a.b" instead of space
	while executing
    "lindex $line 1"
	(procedure "gettreeline" line 9)
	invoked from within
    "gettreeline file11 4b155a05282eeccd7c8fd381b22ed442efde2850"
	("eval" body line 1)
	invoked from within
    "eval $script"
	(procedure "dorunq" line 9)
	invoked from within
    "dorunq"
	("after" script)

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

--

My workaround for the problem was the patch attached, but as I know
next to nothing of Tcl, I suspect it is at least incomplete (there
could be other places with the same problem).

diff --git a/gitk-git/gitk b/gitk-git/gitk
index f1f21e9..3368148 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4946,14 +4946,10 @@ proc gettreeline {gtf id} {
 	if {$diffids eq $nullid} {
 	    set fname $line
 	} else {
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
-	    set i [string first "\t" $line]
-	    if {$i < 0} continue
-	    set sha1 [lindex $line 2]
-	    set fname [string range $line [expr {$i+1}] end]
-	    if {[string index $fname 0] eq "\""} {
-		set fname [lindex $fname 0]
-	    }
+	    set la [split "$line" " \t"]
+	    if {$diffids ne $nullid2 && [lindex $la 1] ne "blob"} continue
+	    set sha1 [lindex $la 2]
+	    set fname [lindex $la 3]
 	    lappend treeidlist($id) $sha1
 	}
 	lappend treefilelist($id) $fname

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] Fix tree mode of the file list for files containing curly brackets
  2008-03-14 21:49 Fix tree mode of the file list for files containing curly brackets Alex Riesen
@ 2008-03-14 21:58 ` Alex Riesen
  2008-04-27 10:57 ` [RESEND] " Alex Riesen
  1 sibling, 0 replies; 5+ messages in thread
From: Alex Riesen @ 2008-03-14 21:58 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Junio C Hamano

As far as I could understand the online documentation the [lindex ...]
thing expects an array, which a string produced by git-ls-tree is not.
So [split ...] it first, to get a real Tcl string-array.

For instance:

    $ git init
    $ date >file
    $ git add . && git commit -m1
    $ git mv file '{a-b}.a.b'
    $ git commit -m2
    $ git mv file '{a-b}.a.b{'
    $ git commit -m3
    $ git mv file '{a-b}.a .b{'
    $ git commit -m4
    $ gitk

Now switch the file list from "Patch" to "Tree":

    list element in braces followed by ".a.b" instead of space
    list element in braces followed by ".a.b" instead of space
	while executing
    "lindex $line 1"
	(procedure "gettreeline" line 9)
	invoked from within
    "gettreeline file11 4b155a05282eeccd7c8fd381b22ed442efde2850"
	("eval" body line 1)
	invoked from within
    "eval $script"
	(procedure "dorunq" line 9)
	invoked from within
    "dorunq"
	("after" script)

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Alex Riesen, Fri, Mar 14, 2008 22:49:04 +0100:
> +	    set la [split "$line" " \t"]
> +	    if {$diffids ne $nullid2 && [lindex $la 1] ne "blob"} continue
> +	    set sha1 [lindex $la 2]
> +	    set fname [lindex $la 3]

And, as I actually do know nothing about Tcl, it does not work for
files with spaces. The last lindex is obviuosly wrong, it breaks names
with whitespaces.

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

diff --git a/gitk-git/gitk b/gitk-git/gitk
index f1f21e9..b3a57be 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4946,14 +4946,10 @@ proc gettreeline {gtf id} {
 	if {$diffids eq $nullid} {
 	    set fname $line
 	} else {
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
-	    set i [string first "\t" $line]
-	    if {$i < 0} continue
-	    set sha1 [lindex $line 2]
-	    set fname [string range $line [expr {$i+1}] end]
-	    if {[string index $fname 0] eq "\""} {
-		set fname [lindex $fname 0]
-	    }
+	    set la [split "$line" " "]
+	    if {$diffids ne $nullid2 && [lindex $la 1] ne "blob"} continue
+	    set sha1 [lindex $la 2]
+	    set fname [lindex [split "$line" "\t"] 1]
 	    lappend treeidlist($id) $sha1
 	}
 	lappend treefilelist($id) $fname
-- 
1.5.4.4.578.g182d

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RESEND] [PATCH] Fix tree mode of the file list for files containing curly brackets
  2008-03-14 21:49 Fix tree mode of the file list for files containing curly brackets Alex Riesen
  2008-03-14 21:58 ` [PATCH] " Alex Riesen
@ 2008-04-27 10:57 ` Alex Riesen
  2008-04-27 11:53   ` Paul Mackerras
  1 sibling, 1 reply; 5+ messages in thread
From: Alex Riesen @ 2008-04-27 10:57 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras, Junio C Hamano

As far as I could understand the online documentation the [lindex ...]
thing expects an array, which a string produced by git-ls-tree is not.
So [split ...] it first, to get a real Tcl string-array.

For instance:

    $ git init
    $ date >file
    $ git add . && git commit -m1
    $ git mv file '{a-b}.a.b'
    $ git commit -m2
    $ git mv file '{a-b}.a.b{'
    $ git commit -m3
    $ git mv file '{a-b}.a .b{'
    $ git commit -m4
    $ gitk

Now switch the file list from "Patch" to "Tree":

    list element in braces followed by ".a.b" instead of space
    list element in braces followed by ".a.b" instead of space
	while executing
    "lindex $line 1"
	(procedure "gettreeline" line 9)
	invoked from within
    "gettreeline file11 4b155a05282eeccd7c8fd381b22ed442efde2850"
	("eval" body line 1)
	invoked from within
    "eval $script"
	(procedure "dorunq" line 9)
	invoked from within
    "dorunq"
	("after" script)

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Alex Riesen, Fri, Mar 14, 2008 22:49:04 +0100:
> +	    set la [split "$line" " \t"]
> +	    if {$diffids ne $nullid2 && [lindex $la 1] ne "blob"} continue
> +	    set sha1 [lindex $la 2]
> +	    set fname [lindex $la 3]

And, as I actually do know nothing about Tcl, it does not work for
files with spaces. The last lindex is obviuosly wrong, it breaks names
with whitespaces.

I rebased the patch on current master.

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

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9a4d9c4..5599878 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4992,14 +4992,10 @@ proc gettreeline {gtf id} {
 	if {$diffids eq $nullid} {
 	    set fname $line
 	} else {
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
-	    set i [string first "\t" $line]
-	    if {$i < 0} continue
-	    set sha1 [lindex $line 2]
-	    set fname [string range $line [expr {$i+1}] end]
-	    if {[string index $fname 0] eq "\""} {
-		set fname [lindex $fname 0]
-	    }
+	    set la [split "$line" " \t"]
+	    if {$diffids ne $nullid2 && [lindex $la 1] ne "blob"} continue
+	    set sha1 [lindex $la 2]
+	    set fname [lindex [split "$line" "\t"] 1]
 	    lappend treeidlist($id) $sha1
 	}
 	lappend treefilelist($id) $fname

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RESEND] [PATCH] Fix tree mode of the file list for files containing curly brackets
  2008-04-27 10:57 ` [RESEND] " Alex Riesen
@ 2008-04-27 11:53   ` Paul Mackerras
  2008-04-27 16:51     ` Alex Riesen
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Mackerras @ 2008-04-27 11:53 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano

Alex Riesen writes:

> As far as I could understand the online documentation the [lindex ...]
> thing expects an array, which a string produced by git-ls-tree is not.
> So [split ...] it first, to get a real Tcl string-array.

Unfortunately that will do the wrong thing if the filename contains a
tab character.  I think the right thing is to split the line textually
at the tab, then treat the first part as a list (which will be OK
since it consists of words without special characters, separated by
spaces), and the second part as the filename.  That is what I was
trying to do anyway, but I forgot to strip off the part after the tab,
which is why lindex got unhappy with it.  Here's the patch I'm about
to commit.

Paul.

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9a4d9c4..da685aa 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4992,11 +4992,12 @@ proc gettreeline {gtf id} {
 	if {$diffids eq $nullid} {
 	    set fname $line
 	} else {
-	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
 	    set i [string first "\t" $line]
 	    if {$i < 0} continue
-	    set sha1 [lindex $line 2]
 	    set fname [string range $line [expr {$i+1}] end]
+	    set line [string range $line 0 [expr {$i-1}]]
+	    if {$diffids ne $nullid2 && [lindex $line 1] ne "blob"} continue
+	    set sha1 [lindex $line 2]
 	    if {[string index $fname 0] eq "\""} {
 		set fname [lindex $fname 0]
 	    }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RESEND] [PATCH] Fix tree mode of the file list for files containing curly brackets
  2008-04-27 11:53   ` Paul Mackerras
@ 2008-04-27 16:51     ` Alex Riesen
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Riesen @ 2008-04-27 16:51 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Junio C Hamano

Paul Mackerras, Sun, Apr 27, 2008 13:53:53 +0200:
> Alex Riesen writes:
> 
> > As far as I could understand the online documentation the [lindex ...]
> > thing expects an array, which a string produced by git-ls-tree is not.
> > So [split ...] it first, to get a real Tcl string-array.
> 
> Unfortunately that will do the wrong thing if the filename contains a
> tab character.  I think the right thing is to split the line textually
> at the tab, then treat the first part as a list (which will be OK
> since it consists of words without special characters, separated by
> spaces), and the second part as the filename.  That is what I was
> trying to do anyway, but I forgot to strip off the part after the tab,
> which is why lindex got unhappy with it.  Here's the patch I'm about
> to commit.
> 

FWIW, it does the right thing for me:

    $ git init
    Initialized empty Git repository in .git/
    $ >a
    $ git add .
    $ gci -m.
    Created initial commit 86ee2fa: .
     0 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 a
    $ git mv a ' { b } '
    $ gci -m.
    Created commit 2a90dc5: .
     1 files changed, 0 insertions(+), 0 deletions(-)
     rename a =>  { b }  (100%)
    $ gitk
    $ happy
    The program 'happy' is currently not installed.  You can install it by typing:
    sudo apt-get install happy
    bash: happy: command not found

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-04-27 16:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-14 21:49 Fix tree mode of the file list for files containing curly brackets Alex Riesen
2008-03-14 21:58 ` [PATCH] " Alex Riesen
2008-04-27 10:57 ` [RESEND] " Alex Riesen
2008-04-27 11:53   ` Paul Mackerras
2008-04-27 16:51     ` Alex Riesen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).