Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: When you do git diff HEAD submodule/, submodule from the index is picked out and match_pathspec_depth() in charge of matching it with the pathspec submodule/. Is ... is called or something missing at the end of this sentence? Unlike tree_entry_interesting(), match_pathspec_depth() has no knowledge about entry mode to realize submodule is a directory and treat the trailing slash specially. And it does not have too, mostly, s/too/to/, I think. because the index only contains files, not directories (not until submodules come) I have no solutions for it (no, stripping '/' at pathspec preprocessing phase seems like a workaround than a solution). So let's mark it. Maybe I or somebody else could revisit it later. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- t/t4010-diff-pathspec.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 15a4912..b54251a 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ submod expect + git diff --name-only HEAD^ submod/ actual I actually doubt that the second line is expecting the right behaviour in the first place. As far as the top-level project is concerned, submod is the name it wants, as there is nothing underneath it. Even if asked to recurse infinite levels, the caller shouldn't be feeding paths like submod/a/b/c to match_pathspec_depth() in the first place, no? + test_cmp expect actual +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Am 23.01.2014 22:09, schrieb Junio C Hamano: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 15a4912..b54251a 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' +git diff --name-only HEAD^ submod expect +git diff --name-only HEAD^ submod/ actual I actually doubt that the second line is expecting the right behaviour in the first place. As far as the top-level project is concerned, submod is the name it wants, as there is nothing underneath it. Even if asked to recurse infinite levels, the caller shouldn't be feeding paths like submod/a/b/c to match_pathspec_depth() in the first place, no? Agreed, submod/a/b/c would not make sense here. But a single trailing '/' does mark submod as a directory, which I think is ok for a submodule. And it makes life easier for the user if we accept that, as shell completion will add it there automatically. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Jens Lehmann jens.lehm...@web.de writes: ... But a single trailing '/' does mark submod as a directory, which I think is ok for a submodule. And it makes life easier for the user if we accept that, as shell completion will add it there automatically. OK, that would be annoying. Perhaps the completion is what is broken here, then? I dunno, and haven't thought things through. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
On Fri, Jan 24, 2014 at 4:38 AM, Junio C Hamano gits...@pobox.com wrote: Jens Lehmann jens.lehm...@web.de writes: ... But a single trailing '/' does mark submod as a directory, which I think is ok for a submodule. And it makes life easier for the user if we accept that, as shell completion will add it there automatically. OK, that would be annoying. Perhaps the completion is what is broken here, then? I dunno, and haven't thought things through. My reasoning is more simple minded: if git diff HEAD HEAD^ submod/ works, the user would expect git diff HEAD submod/ to work too. I think I've got something working with a bit refactoring. Will send out soon. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
