Linus Torvalds [off-list ref] writes:
You're right, we need to keep the "-r" behaviour.
If you take my last patch and make "ls_flags" default to 0 again, it
should work correctly.
Maybe making "-r" the default and resurrecting "-d" to turn it
off would be the approach of least impact (-r becomes a
noiseword), if we take this "ls-tree should parallel ls-files"
idea.
The merge-recursive strategy uses ls-tree without -r with path
specifier to grab the sha1/mode of the three trees involved.
git-cvsexportcommit has the same assumption on how ls-tree
works. It extracts a single SHA1 for the blob from the commit
object by "ls-tree $commit $blob_path".
With your two patches, they need to do this with "-r".
git-checkout and git-reset use -r to grab the whole tree, so
they are fine either way. git-svnimport uses -r to read from a
tree (potentially a subtree) all the way down, so this one is
also fine.
So it probably is safer to default ls_options to LS_RECURSIVE,
and use "-d" to restrict it not to recurse.
But after seeing some more examples, I tend to think the current
one that models after how "/bin/ls -a" works in a way that is a
lot easier to understand. For example, with your two patches
(defaulting to recursive)
git-ls-tree HEAD
shows everything from the tree. In order to get prettyprint of
HEAD tree (i.e. single level listing):
git-ls-tree -d HEAD
is needed. But there is no way to get the single level with
pathspec with these patches, so:
cd Documentation && git-ls-tree -d HEAD
would not show the single level of Documentation tree, but just
a single line "tree" object of Documentation tree. Which means
the way it works from the top and the way it works in a
subdirectory is quite different.
I'll throw in your two patches to the proposed updates branch so
that Porcelain/Browser people can play with it and decide. I
really love the decrease of number of lines of code, but I am
afraid this would end up breaking things without real merit.
On Sun, 27 Nov 2005, Junio C Hamano wrote:
So it probably is safer to default ls_options to LS_RECURSIVE,
and use "-d" to restrict it not to recurse.
I have a patch that solves all the problems, I think.
It modifies the selection a bit, so that a pathspec that is a superset of
a particular tree path will always cause it to recurse into that tree.
As an example, let's say that we do
git-ls-tree HEAD drivers/char
_without_ the "-r". What will happen is that it will start out doing all
the base tree, and for "drivers" it will notice that it's a proper subset
of "drivers/char", so it will always recurse into _that_ tree (but not
into other trees).
Then, it will not match anything else than "char" in that subdirectory,
and because that's not a proper superset (it's an exact match), it will
_not_ recurse into it, so you get:
[torvalds@g5 linux]$ ~/git/git-ls-tree HEAD drivers/char
040000 tree 9568cda453aae205bb58983747fa73b9696d9d51 drivers/char
which is what you got with the old git-ls-tree too.
But interestingly, if you add the slash, it will become a proper superset
and it will recurse into _that_ subdirectory (but no deeper: so if you
want all subdirectories _below_ drivers/char/, you still need to give
"-r"):
[torvalds@g5 linux]$ ~/git/git-ls-tree HEAD drivers/char/
100644 blob 2b6b1d772ed776fff87927fc34adc2e40500218e drivers/char/.gitignore
100644 blob 56b8a2e76ab10a5c21787cb7068a846075cbaffd drivers/char/ChangeLog
100644 blob 970f70d498f4c814e1cf3362e33d7e23ac53c299 drivers/char/Kconfig
...
See? This is on top of the previous two diffs, holler if you want a whole
new "everything combined" version..
It hasn't gotten lots of testing, but it should work.
BTW! It fails a few of the tests, but it does so because now, with "-r",
it will never show the tree entries, and because of the difference between
git-ls-tree $tree path
and
git-ls-tree $tree path/
that it didn't use to have (oh, and if you give multiple paths, it will
now always consider them a "union pathspec", not an "iteration of paths").
But I _think_ the new behaviour is pretty useful and while not the same as
"ls", it's perhaps still intuitive enough..
Linus
---
diff --git a/ls-tree.c b/ls-tree.c
index cf0dbbc..4df5830 100644
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -11,7 +11,8 @@
static int line_termination = '\n';
#define LS_RECURSIVE 1
#define LS_TREE_ONLY 2
-static int ls_options = LS_RECURSIVE;
+static int ls_options = 0;
+const char **pathspec;
static const char ls_tree_usage[] =
"git-ls-tree [-d] [-r] [-z] <tree-ish> [path...]";
@@ -21,8 +22,29 @@ static int show_tree(unsigned char *sha1
const char *type = "blob";
if (S_ISDIR(mode)) {
+ const char **s;
if (ls_options & LS_RECURSIVE)
return READ_TREE_RECURSIVE;
+ s = pathspec;
+ if (s) {
+ for (;;) {
+ const char *spec = *s++;
+ int len, speclen;
+
+ if (!spec)
+ break;
+ if (strncmp(base, spec, baselen))
+ continue;
+ len = strlen(pathname);
+ spec += baselen;
+ speclen = strlen(spec);
+ if (speclen <= len)
+ continue;
+ if (memcmp(pathname, spec, len))
+ continue;
+ return READ_TREE_RECURSIVE;
+ }
+ }
type = "tree";
}
@@ -32,7 +54,7 @@ static int show_tree(unsigned char *sha1
int main(int argc, const char **argv)
{
- const char **path, *prefix;
+ const char *prefix;
unsigned char sha1[20];
char *buf;
unsigned long size;@@ -60,11 +82,11 @@ int main(int argc, const char **argv)
if (get_sha1(argv[1], sha1) < 0)
usage(ls_tree_usage);
- path = get_pathspec(prefix, argv + 2);
+ pathspec = get_pathspec(prefix, argv + 2);
buf = read_object_with_reference(sha1, "tree", &size, NULL);
if (!buf)
die("not a tree object");
- read_tree_recursive(buf, size, "", 0, 0, path, show_tree);
+ read_tree_recursive(buf, size, "", 0, 0, pathspec, show_tree);
return 0;
}