[PATCH] gitk: fix the display of files when filtered by path

Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
under the given directory but the file list is left empty. This is because
the path_filter function fails to match the filenames which are relative
to the working tree to the filter which is filessytem relative.
This solves the problem by making both names fully qualified filesystem
paths before performing the comparison.

Signed-off-by: Pat Thoyts <redacted>
---
 gitk-git/gitk |   38 +++++++++++++++++++++++++++-----------
 1 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 2a92e20..b728345 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk

@@ -18,6 +18,26 @@ proc gitdir {} {
     }
 }
 
+proc gitworktree {} {
+    variable _gitworktree
+    if {[info exists _gitworktree]} {
+	return $_gitworktree
+    }
+    # v1.7.0 introduced --show-toplevel to return the canonical work-tree
+    if {[catch {set _gitworktree [exec git rev-parse --show-toplevel]}]} {
+        # try to set work tree from environment, core.worktree or use
+        # cdup to obtain a relative path to the top of the worktree. If
+        # run from the top, the ./ prefix ensures normalize expands pwd.
+        if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
+	    catch {set _gitworktree [exec git config --get core.worktree]}
+	    if {$_gitworktree eq ""} {
+		set _gitworktree [file normalize ./[exec git rev-parse --show-cdup]]
+	    }
+        }
+    }
+    return $_gitworktree
+}
+
 # A simple scheduler for compute-intensive stuff.
 # The aim is to make sure that event handlers for GUI actions can
 # run at least every 50-100 ms.  Unfortunately fileevent handlers are

@@ -7376,19 +7396,15 @@ proc startdiff {ids} {
     }
 }
 
+# If the filename (name) is under any of the passed filter paths
+# then return true to include the file in the listing.
 proc path_filter {filter name} {
+    set worktree [gitworktree]
     foreach p $filter {
-	set l [string length $p]
-	if {[string index $p end] eq "/"} {
-	    if {[string compare -length $l $p $name] == 0} {
-		return 1
-	    }
-	} else {
-	    if {[string compare -length $l $p $name] == 0 &&
-		([string length $name] == $l ||
-		 [string index $name $l] eq "/")} {
-		return 1
-	    }
+	set fq_p [file normalize $p]
+	set fq_n [file normalize [file join $worktree $name]]
+	if {[string match [file normalize $fq_p]* $fq_n]} {
+	    return 1
 	}
     }
     return 0

-- 
1.7.8.msysgit.0

On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
[off-list ref] wrote:

Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
under the given directory but the file list is left empty. This is because
the path_filter function fails to match the filenames which are relative
to the working tree to the filter which is filessytem relative.
This solves the problem by making both names fully qualified filesystem
paths before performing the comparison.

Signed-off-by: Pat Thoyts <redacted>

Wonderful!

I've run into this problem a number of times (as have some co-workers)
but I never bothered to report it since I felt guilty for never having
worked up a patch.

I tested this and it worked.

FWIW,

Tested-by: David Aguilar <redacted>


Thank you Pat!
-- 
            David

Hi,

On Wed, 14 Dec 2011, David Aguilar wrote:

On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
[off-list ref] wrote:

quoted

Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
under the given directory but the file list is left empty. This is because
the path_filter function fails to match the filenames which are relative
to the working tree to the filter which is filessytem relative.
This solves the problem by making both names fully qualified filesystem
paths before performing the comparison.

Signed-off-by: Pat Thoyts <redacted>

Wonderful!

Thanks for reminding me that I did not yet apply and push. Did so now.

Thanks, Pat, for fixing this bug!
Dscho

Hi,

On Thu, Dec 15, 2011 at 1:24 AM, Johannes Schindelin
[off-list ref] wrote:

Hi,

On Wed, 14 Dec 2011, David Aguilar wrote:

quoted

On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
[off-list ref] wrote:

quoted

Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
under the given directory but the file list is left empty. This is because
the path_filter function fails to match the filenames which are relative
to the working tree to the filter which is filessytem relative.
This solves the problem by making both names fully qualified filesystem
paths before performing the comparison.

How is this related to my patches from April? See
http://thread.gmane.org/gmane.comp.version-control.git/170853. It's
clearly not the same problem, but will the patches conflict? Will some
of mine be unnecessary?

Thanks for reminding me that I did not yet apply and push. Did so now.

What do you mean by this? Push to where?
git://git.kernel.org/pub/scm/gitk/gitk.git is still down.

Paul and Junio, the patches I sent in April are still not in git.git,
are they? Can we use another repo until the kernel.org one is up? More
than eight months to get a patch (or eight) merged is way too long,
IMO.

Martin

On Thu, Dec 15, 2011 at 11:42:38AM -0800, Martin von Zweigbergk wrote:

git://git.kernel.org/pub/scm/gitk/gitk.git is still down.

I have just created a repository on ozlabs.org for gitk, since I don't
have kernel.org access at this point.  The repository is:

git://ozlabs.org/~paulus/gitk.git

Paul and Junio, the patches I sent in April are still not in git.git,
are they? Can we use another repo until the kernel.org one is up? More
than eight months to get a patch (or eight) merged is way too long,
IMO.

Your patches are in the master branch.  I applied them back in July
but then kernel.org went down.

Paul.

On Tue, Dec 13, 2011 at 04:50:50PM +0000, Pat Thoyts wrote:

Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
under the given directory but the file list is left empty. This is because
the path_filter function fails to match the filenames which are relative
to the working tree to the filter which is filessytem relative.
This solves the problem by making both names fully qualified filesystem
paths before performing the comparison.

Signed-off-by: Pat Thoyts <redacted>

Thanks, applied.

Paul.