[GITK PATCH] gitk: support "gitk <tracheophyte> -- ."

[GITK PATCH] gitk: support "gitk <tracheophyte> -- ."

[Johannes Schindelin]

It might be unintuitive to a user when "gitk HEAD -- ." does not show
any files in the lower right pane. This patch fixes that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	My initial analysis was all wrong.

	Pat: does this look correct to you?

	Kirill: does this fix the issue on your side, too?

	Paul: since we need this for Git Cheetah, and you are probably too 
	busy to apply/review in the next few weeks, I took the liberty of 
	keeping it as a git.git patch. Let me know if I you want it 
	in a different form that does not require git-am's -p2 option.

 gitk-git/gitk |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index cdedaa7..553922f 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -7341,6 +7341,9 @@ proc startdiff {ids} {
 
 proc path_filter {filter name} {
     foreach p $filter {
+	if {$p == "."} {
+		return 1
+	}
 	set l [string length $p]
 	if {[string index $p end] eq "/"} {
 	    if {[string compare -length $l $p $name] == 0} {
-- 
1.6.4.297.gcb4cc

[GITK PATCH 2/3] gitk: support path filters even in subdirectories

[Johannes Schindelin]

Even when running inside a subdirectory, "gitk HEAD -- ." should work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 gitk-git/gitk |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 553922f..bad9ebc 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -7340,9 +7340,12 @@ proc startdiff {ids} {
 }
 
 proc path_filter {filter name} {
+    global pathprefix
     foreach p $filter {
 	if {$p == "."} {
-		return 1
+		set p $pathprefix
+	} else {
+		set p $pathprefix$p
 	}
 	set l [string length $p]
 	if {[string index $p end] eq "/"} {
@@ -11585,6 +11588,7 @@ readrefs
 
 if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {
     # create a view for the files/dirs specified on the command line
+    set pathprefix [exec git rev-parse --show-prefix]
     set curview 1
     set selectedview 1
     set nextviewnum 2
-- 
1.6.4.297.gcb4cc

[GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories

[Johannes Schindelin]

Again in the lower right panel, where the file names of the files touched
by the current commit are clickable: let's not show the prefix when we
are in a subdirectory, as it wastes precious screen estate conveying
information the user already knows.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Sorry, I tested 1/3 only in the gitk-git/ subdirectory. And of 
	course, I was bitten by the fact that this subdirectory is pulled
	using the subtree strategy, so there are files in the history 
	which lack the prefix. Therefore, I saw the files even if the fix
	was incomplete.

 gitk-git/gitk |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index bad9ebc..0b2c351 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3203,10 +3203,14 @@ proc unhighlight_filelist {} {
 }
 
 proc add_flist {fl} {
-    global cflist
+    global cflist pathprefix
 
     $cflist conf -state normal
+    set l [string length $pathprefix]
     foreach f $fl {
+        if {$l > 0 && [string compare -length $l $pathprefix $f] == 0} {
+	    set f [string range $f $l end]
+	}
 	$cflist insert end "\n"
 	$cflist insert end $f [highlight_tag $f]
     }
-- 
1.6.4.297.gcb4cc

Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories

[Kirill]

Hi,

On Tue, Feb 23, 2010 at 12:12 PM, Johannes Schindelin wrote:
>
> Again in the lower right panel, where the file names of the files touched
> by the current commit are clickable: let's not show the prefix when we
> are in a subdirectory, as it wastes precious screen estate conveying
> information the user already knows.
Unfortunately, it seems to be too aggressive, leading to a misleading
display. When gitk is invoked from a subdirectory but without the
filter, the lower right panel displays some paths, relative to the
root of the work tree, and some, relative to the wd:

$ # fresh netinstall with checked out devel's
$ cd /
$ gitk --all & # 1
$ cd /bin
$ gitk --all & # 2

1. bin/move-wiki.sh when devel is selected; share/WinGit/install.iss -
when installer-improvements is selected (that's correct)

2. move-wiki.sh on devel; share/WinGit/install.iss on installer-improvements
That's misleading.

And honestly, I'm not that advanced in gitk use, so somebody will
probably have to do some more testing.

Thanks!

--
Kirill.

Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Kirill wrote:

> Unfortunately, it seems to be too aggressive, leading to a misleading 
> display. When gitk is invoked from a subdirectory but without the 
> filter, the lower right panel displays some paths, relative to the root 
> of the work tree, and some, relative to the wd:

Right. I fixed it in 2/3: pathprefix is set to "" when no cmdline_files 
were specified (i.e. when there is no filter). An when pathprefix is "", 
nothing changes to the situation before.

Good?

Ciao,
Dscho

Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories

[Kirill]

Hi Dscho,

On Tue, Feb 23, 2010 at 3:50 PM, Johannes Schindelin wrote:
> On Tue, 23 Feb 2010, Kirill wrote:
>
>> Unfortunately, it seems to be too aggressive, leading to a misleading
>> display. When gitk is invoked from a subdirectory but without the
>> filter, the lower right panel displays some paths, relative to the root
>> of the work tree, and some, relative to the wd:
>
> Right. I fixed it in 2/3: pathprefix is set to "" when no cmdline_files
> were specified (i.e. when there is no filter). An when pathprefix is "",
> nothing changes to the situation before.
>
> Good?
Seems to work for me. Thank you!

Pat, Paul, like I said, you would not want to take my testing seriously.

--
Kirill.

Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories

[Kirill]

Hi,

Dscho, at first, thank you so much for working on the issue!
In general the series work. At least, it passes my limited testing
from the original message. However...

On Tue, Feb 23, 2010 at 12:10 PM, Johannes Schindelin wrote:
>
> Even when running inside a subdirectory, "gitk HEAD -- ." should work.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  gitk-git/gitk |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 553922f..bad9ebc 100644
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -7340,9 +7340,12 @@ proc startdiff {ids} {
>  }
>
>  proc path_filter {filter name} {
> +    global pathprefix
>     foreach p $filter {
>        if {$p == "."} {
> -               return 1
> +               set p $pathprefix
> +       } else {
> +               set p $pathprefix$p
>        }
>        set l [string length $p]
>        if {[string index $p end] eq "/"} {
> @@ -11585,6 +11588,7 @@ readrefs
>
>  if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {
>     # create a view for the files/dirs specified on the command line
> +    set pathprefix [exec git rev-parse --show-prefix]
I believe the fact that pathprefix is set only under several
conditions, the invocation without arguments is broken.

My .02

--
Kirill.

Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Kirill wrote:

> I believe the fact that pathprefix is set only under several conditions, 
> the invocation without arguments is broken.

You are absolutely correct!

Will fix and push to work/gitk-dashdash-dot,
Dscho

Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories

[Pat Thoyts]

On 23 February 2010 20:22, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Tue, 23 Feb 2010, Kirill wrote:
>
>> I believe the fact that pathprefix is set only under several conditions,
>> the invocation without arguments is broken.
>
> You are absolutely correct!
>
> Will fix and push to work/gitk-dashdash-dot,
> Dscho

This doesn't seem to work for me. We are trying to have the file tree
window display filenames when 'gitk -- .' is used and with your patch
this isn't happening when I apply this to gitk. I broke out the
path_filter function into a separate test to play with it a bit. It
seems this function is trying to match a path prefix to the provided
file name so here is a test script with three implementations. The
original, dscho's new one (git rev-parse --show-prefix returns an
empty string when run in the toplevel directory so I force the
'pathprefix' variable for the tests).

With this script I get the following results:
C:\src\gitk>tclsh told.tcl
original-2 failed . gitk expected 1 got 0
original-3 failed ./ gitk expected 1 got 0
original-5 failed ./po po/de.po expected 1 got 0
dscho-2 failed . gitk expected 1 got 0
dscho-3 failed ./ gitk expected 1 got 0
dscho-5 failed ./po po/de.po expected 1 got 0

So it looks like a simple string match on a normalized path works ok.
[file normalize $name] doesn't require the target file to exists btw.

--- test script begins ---

proc path_filter_orig {filter name} {
    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
	    }
	}
    }
    return 0
}

proc path_filter_dscho {filter name} {
    set pathprefix ""
    foreach p $filter {
        if {$p == "."} {
            set p $pathprefix
        } else {
            set p $pathprefix$p
        }
        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
	    }
	}
    }
    return 0
}

proc path_filter {filter name} {
    set name [file normalize $name]
    foreach p $filter {
        set p [file normalize $p]
        if {[string equal $p $name] || [string match $p* $name]} {
            return 1
        }
    }
    return 0
}

set tests {
    1  ""   gitk   0
    2  .    gitk   1
    3  ./   gitk   1
    4  po   po/de.po  1
    5  ./po po/de.po 1
    6  po   gitk   0
    7  po   a/b    0
    8  a    a/b/c  1
}

foreach {id filter name result} $tests {
    set testresult [path_filter_orig $filter $name]
    if {$testresult != $result} {
        puts "original-$id failed $filter $name expected $result got
$testresult"
    }
}

foreach {id filter name result} $tests {
    set testresult [path_filter_dscho $filter $name]
    if {$testresult != $result} {
        puts "dscho-$id failed $filter $name expected $result got $testresult"
    }
}

foreach {id filter name result} $tests {
    set testresult [path_filter $filter $name]
    if {$testresult != $result} {
        puts "new-$id failed $filter $name expected $result got $testresult"
    }
}

Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories

[Johannes Schindelin]

Hi,

On Thu, 25 Feb 2010, Pat Thoyts wrote:

> proc path_filter {filter name} {
>     set name [file normalize $name]

I am unconvinced that [file normalize] is what we want. It expands to an 
absolute path, which is almost certainly unnecessary code churn.

I will have a look at your test cases and try to fix later.

Ciao,
Johannes