{"thread":{"id":"22769","subject":"[GITK PATCH] gitk: support \"gitk -- .\"","startedAt":"2010-02-23T16:51:46Z","lastAt":"2010-02-25T14:22:17Z","messageCount":10,"participants":["Johannes Schindelin","Kirill","Pat Thoyts"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"135436","messageId":"alpine.DEB.1.00.1002231748320.3980@intel-tinevez-2-302","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002201920350.20986@pacific.mpi-cbg.de","subject":"[GITK PATCH] gitk: support \"gitk -- .\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-23T16:51:46Z","receivedAt":"2010-02-23T16:51:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt might be unintuitive to a user when \"gitk HEAD -- .\" does not show\nany files in the lower right pane. This patch fixes that.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tMy initial analysis was all wrong.\n\n\tPat: does this look correct to you?\n\n\tKirill: does this fix the issue on your side, too?\n\n\tPaul: since we need this for Git Cheetah, and you are probably too \n\tbusy to apply/review in the next few weeks, I took the liberty of \n\tkeeping it as a git.git patch. Let me know if I you want it \n\tin a different form that does not require git-am's -p2 option.\n\n gitk-git/gitk | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex cdedaa7..553922f 100644\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -7341,6 +7341,9 @@ proc startdiff {ids} {\n \n proc path_filter {filter name} {\n foreach p $filter {\n+\tif {$p == \".\"} {\n+\t\treturn 1\n+\t}\n \tset l [string length $p]\n \tif {[string index $p end] eq \"/\"} {\n \t if {[string compare -length $l $p $name] == 0} {\n-- \n1.6.4.297.gcb4cc\n"},{"id":"135441","messageId":"alpine.DEB.1.00.1002231810020.3980@intel-tinevez-2-302","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002231748320.3980@intel-tinevez-2-302","subject":"[GITK PATCH 2/3] gitk: support path filters even in subdirectories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-23T17:10:18Z","receivedAt":"2010-02-23T17:10:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEven when running inside a subdirectory, \"gitk HEAD -- .\" should work.\n\nSigned-off-by: Johannes Schindelin \n---\n gitk-git/gitk | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 553922f..bad9ebc 100644\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -7340,9 +7340,12 @@ proc startdiff {ids} {\n }\n \n proc path_filter {filter name} {\n+ global pathprefix\n foreach p $filter {\n \tif {$p == \".\"} {\n-\t\treturn 1\n+\t\tset p $pathprefix\n+\t} else {\n+\t\tset p $pathprefix$p\n \t}\n \tset l [string length $p]\n \tif {[string index $p end] eq \"/\"} {\n@@ -11585,6 +11588,7 @@ readrefs\n \n if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {\n # create a view for the files/dirs specified on the command line\n+ set pathprefix [exec git rev-parse --show-prefix]\n set curview 1\n set selectedview 1\n set nextviewnum 2\n-- \n1.6.4.297.gcb4cc\n"},{"id":"135443","messageId":"alpine.DEB.1.00.1002231811021.3980@intel-tinevez-2-302","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002231810020.3980@intel-tinevez-2-302","subject":"[GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-23T17:12:35Z","receivedAt":"2010-02-23T17:12:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nAgain in the lower right panel, where the file names of the files touched\nby the current commit are clickable: let's not show the prefix when we\nare in a subdirectory, as it wastes precious screen estate conveying\ninformation the user already knows.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tSorry, I tested 1/3 only in the gitk-git/ subdirectory. And of \n\tcourse, I was bitten by the fact that this subdirectory is pulled\n\tusing the subtree strategy, so there are files in the history \n\twhich lack the prefix. Therefore, I saw the files even if the fix\n\twas incomplete.\n\n gitk-git/gitk | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex bad9ebc..0b2c351 100644\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -3203,10 +3203,14 @@ proc unhighlight_filelist {} {\n }\n \n proc add_flist {fl} {\n- global cflist\n+ global cflist pathprefix\n \n $cflist conf -state normal\n+ set l [string length $pathprefix]\n foreach f $fl {\n+ if {$l > 0 && [string compare -length $l $pathprefix $f] == 0} {\n+\t set f [string range $f $l end]\n+\t}\n \t$cflist insert end \"\\n\"\n \t$cflist insert end $f [highlight_tag $f]\n }\n-- \n1.6.4.297.gcb4cc\n"},{"id":"135452","messageId":"f579dd581002231137t71bb034fl429fd03a2c0d681c@mail.gmail.com","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002231810020.3980@intel-tinevez-2-302","subject":"Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories","fromName":"Kirill","fromEmail":"kirillathome@gmail.com","sentAt":"2010-02-23T19:37:37Z","receivedAt":"2010-02-23T19:37:37Z","isPatch":true,"sender":{"key":"kirillathome@gmail.com","avatar":null},"body":"Hi,\n\nDscho, at first, thank you so much for working on the issue!\nIn general the series work. At least, it passes my limited testing\nfrom the original message. However...\n\nOn Tue, Feb 23, 2010 at 12:10 PM, Johannes Schindelin wrote:\n>\n> Even when running inside a subdirectory, \"gitk HEAD -- .\" should work.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n>  gitk-git/gitk |    6 +++++-\n>  1 files changed, 5 insertions(+), 1 deletions(-)\n>\n> diff --git a/gitk-git/gitk b/gitk-git/gitk\n> index 553922f..bad9ebc 100644\n> --- a/gitk-git/gitk\n> +++ b/gitk-git/gitk\n> @@ -7340,9 +7340,12 @@ proc startdiff {ids} {\n>  }\n>\n>  proc path_filter {filter name} {\n> +    global pathprefix\n>     foreach p $filter {\n>        if {$p == \".\"} {\n> -               return 1\n> +               set p $pathprefix\n> +       } else {\n> +               set p $pathprefix$p\n>        }\n>        set l [string length $p]\n>        if {[string index $p end] eq \"/\"} {\n> @@ -11585,6 +11588,7 @@ readrefs\n>\n>  if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {\n>     # create a view for the files/dirs specified on the command line\n> +    set pathprefix [exec git rev-parse --show-prefix]\nI believe the fact that pathprefix is set only under several\nconditions, the invocation without arguments is broken.\n\nMy .02\n\n--\nKirill.\n"},{"id":"135453","messageId":"f579dd581002231142v6a937ac0xdc9618f2a468989d@mail.gmail.com","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002231811021.3980@intel-tinevez-2-302","subject":"Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories","fromName":"Kirill","fromEmail":"kirillathome@gmail.com","sentAt":"2010-02-23T19:42:29Z","receivedAt":"2010-02-23T19:42:29Z","isPatch":true,"sender":{"key":"kirillathome@gmail.com","avatar":null},"body":"Hi,\n\nOn Tue, Feb 23, 2010 at 12:12 PM, Johannes Schindelin wrote:\n>\n> Again in the lower right panel, where the file names of the files touched\n> by the current commit are clickable: let's not show the prefix when we\n> are in a subdirectory, as it wastes precious screen estate conveying\n> information the user already knows.\nUnfortunately, it seems to be too aggressive, leading to a misleading\ndisplay. When gitk is invoked from a subdirectory but without the\nfilter, the lower right panel displays some paths, relative to the\nroot of the work tree, and some, relative to the wd:\n\n$ # fresh netinstall with checked out devel's\n$ cd /\n$ gitk --all & # 1\n$ cd /bin\n$ gitk --all & # 2\n\n1. bin/move-wiki.sh when devel is selected; share/WinGit/install.iss -\nwhen installer-improvements is selected (that's correct)\n\n2. move-wiki.sh on devel; share/WinGit/install.iss on installer-improvements\nThat's misleading.\n\nAnd honestly, I'm not that advanced in gitk use, so somebody will\nprobably have to do some more testing.\n\nThanks!\n\n--\nKirill.\n"},{"id":"135458","messageId":"alpine.DEB.1.00.1002232122110.3980@intel-tinevez-2-302","threadId":"22769","inReplyTo":"f579dd581002231137t71bb034fl429fd03a2c0d681c@mail.gmail.com","subject":"Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-23T20:22:44Z","receivedAt":"2010-02-23T20:22:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 23 Feb 2010, Kirill wrote:\n\n> I believe the fact that pathprefix is set only under several conditions, \n> the invocation without arguments is broken.\n\nYou are absolutely correct!\n\nWill fix and push to work/gitk-dashdash-dot,\nDscho\n"},{"id":"135459","messageId":"alpine.DEB.1.00.1002232148470.3980@intel-tinevez-2-302","threadId":"22769","inReplyTo":"f579dd581002231142v6a937ac0xdc9618f2a468989d@mail.gmail.com","subject":"Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-23T20:50:21Z","receivedAt":"2010-02-23T20:50:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 23 Feb 2010, Kirill wrote:\n\n> Unfortunately, it seems to be too aggressive, leading to a misleading \n> display. When gitk is invoked from a subdirectory but without the \n> filter, the lower right panel displays some paths, relative to the root \n> of the work tree, and some, relative to the wd:\n\nRight. I fixed it in 2/3: pathprefix is set to \"\" when no cmdline_files \nwere specified (i.e. when there is no filter). An when pathprefix is \"\", \nnothing changes to the situation before.\n\nGood?\n\nCiao,\nDscho\n"},{"id":"135483","messageId":"f579dd581002231420l2fd81b19n5b1cf2887ada2871@mail.gmail.com","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002232148470.3980@intel-tinevez-2-302","subject":"Re: [GITK PATCH 3/3] gitk: strip prefix from filenames in subdirectories","fromName":"Kirill","fromEmail":"kirillathome@gmail.com","sentAt":"2010-02-23T22:20:57Z","receivedAt":"2010-02-23T22:20:57Z","isPatch":true,"sender":{"key":"kirillathome@gmail.com","avatar":null},"body":"Hi Dscho,\n\nOn Tue, Feb 23, 2010 at 3:50 PM, Johannes Schindelin wrote:\n> On Tue, 23 Feb 2010, Kirill wrote:\n>\n>> Unfortunately, it seems to be too aggressive, leading to a misleading\n>> display. When gitk is invoked from a subdirectory but without the\n>> filter, the lower right panel displays some paths, relative to the root\n>> of the work tree, and some, relative to the wd:\n>\n> Right. I fixed it in 2/3: pathprefix is set to \"\" when no cmdline_files\n> were specified (i.e. when there is no filter). An when pathprefix is \"\",\n> nothing changes to the situation before.\n>\n> Good?\nSeems to work for me. Thank you!\n\nPat, Paul, like I said, you would not want to take my testing seriously.\n\n--\nKirill.\n"},{"id":"135646","messageId":"a5b261831002241751v5294af48rac8b5f52ba6cb045@mail.gmail.com","threadId":"22769","inReplyTo":"alpine.DEB.1.00.1002232122110.3980@intel-tinevez-2-302","subject":"Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories","fromName":"Pat Thoyts","fromEmail":"patthoyts@googlemail.com","sentAt":"2010-02-25T01:51:31Z","receivedAt":"2010-02-25T01:51:31Z","isPatch":true,"sender":{"key":"patthoyts@googlemail.com","avatar":"https://gravatar.com/avatar/2300f94d9f59174a551dbc71278ac2cb2489f56afda920a57e189ad9de0a3f92?d=mp&s=160"},"body":"On 23 February 2010 20:22, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Tue, 23 Feb 2010, Kirill wrote:\n>\n>> I believe the fact that pathprefix is set only under several conditions,\n>> the invocation without arguments is broken.\n>\n> You are absolutely correct!\n>\n> Will fix and push to work/gitk-dashdash-dot,\n> Dscho\n\nThis doesn't seem to work for me. We are trying to have the file tree\nwindow display filenames when 'gitk -- .' is used and with your patch\nthis isn't happening when I apply this to gitk. I broke out the\npath_filter function into a separate test to play with it a bit. It\nseems this function is trying to match a path prefix to the provided\nfile name so here is a test script with three implementations. The\noriginal, dscho's new one (git rev-parse --show-prefix returns an\nempty string when run in the toplevel directory so I force the\n'pathprefix' variable for the tests).\n\nWith this script I get the following results:\nC:\\src\\gitk>tclsh told.tcl\noriginal-2 failed . gitk expected 1 got 0\noriginal-3 failed ./ gitk expected 1 got 0\noriginal-5 failed ./po po/de.po expected 1 got 0\ndscho-2 failed . gitk expected 1 got 0\ndscho-3 failed ./ gitk expected 1 got 0\ndscho-5 failed ./po po/de.po expected 1 got 0\n\nSo it looks like a simple string match on a normalized path works ok.\n[file normalize $name] doesn't require the target file to exists btw.\n\n--- test script begins ---\n\nproc path_filter_orig {filter name} {\n foreach p $filter {\n set l [string length $p]\n\tif {[string index $p end] eq \"/\"} {\n\t if {[string compare -length $l $p $name] == 0} {\n\t\treturn 1\n\t }\n\t} else {\n\t if {[string compare -length $l $p $name] == 0 &&\n\t\t([string length $name] == $l ||\n\t\t [string index $name $l] eq \"/\")} {\n\t\treturn 1\n\t }\n\t}\n }\n return 0\n}\n\nproc path_filter_dscho {filter name} {\n set pathprefix \"\"\n foreach p $filter {\n if {$p == \".\"} {\n set p $pathprefix\n } else {\n set p $pathprefix$p\n }\n set l [string length $p]\n\tif {[string index $p end] eq \"/\"} {\n\t if {[string compare -length $l $p $name] == 0} {\n\t\treturn 1\n\t }\n\t} else {\n\t if {[string compare -length $l $p $name] == 0 &&\n\t\t([string length $name] == $l ||\n\t\t [string index $name $l] eq \"/\")} {\n\t\treturn 1\n\t }\n\t}\n }\n return 0\n}\n\nproc path_filter {filter name} {\n set name [file normalize $name]\n foreach p $filter {\n set p [file normalize $p]\n if {[string equal $p $name] || [string match $p* $name]} {\n return 1\n }\n }\n return 0\n}\n\nset tests {\n 1 \"\" gitk 0\n 2 . gitk 1\n 3 ./ gitk 1\n 4 po po/de.po 1\n 5 ./po po/de.po 1\n 6 po gitk 0\n 7 po a/b 0\n 8 a a/b/c 1\n}\n\nforeach {id filter name result} $tests {\n set testresult [path_filter_orig $filter $name]\n if {$testresult != $result} {\n puts \"original-$id failed $filter $name expected $result got\n$testresult\"\n }\n}\n\nforeach {id filter name result} $tests {\n set testresult [path_filter_dscho $filter $name]\n if {$testresult != $result} {\n puts \"dscho-$id failed $filter $name expected $result got $testresult\"\n }\n}\n\nforeach {id filter name result} $tests {\n set testresult [path_filter $filter $name]\n if {$testresult != $result} {\n puts \"new-$id failed $filter $name expected $result got $testresult\"\n }\n}\n"},{"id":"135676","messageId":"alpine.DEB.1.00.1002251427110.3869@intel-tinevez-2-302","threadId":"22769","inReplyTo":"a5b261831002241751v5294af48rac8b5f52ba6cb045@mail.gmail.com","subject":"Re: [GITK PATCH 2/3] gitk: support path filters even in subdirectories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-25T14:22:17Z","receivedAt":"2010-02-25T14:22:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 25 Feb 2010, Pat Thoyts wrote:\n\n> proc path_filter {filter name} {\n> set name [file normalize $name]\n\nI am unconvinced that [file normalize] is what we want. It expands to an \nabsolute path, which is almost certainly unnecessary code churn.\n\nI will have a look at your test cases and try to fix later.\n\nCiao,\nJohannes\n"}]}