* [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
@ 2010-09-09 18:21 davi.reis
2010-09-09 18:21 ` [PATCH 2/2] Added test from David Glasser davi.reis
2010-09-09 21:30 ` [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list Matthieu Moy
0 siblings, 2 replies; 5+ messages in thread
From: davi.reis @ 2010-09-09 18:21 UTC (permalink / raw)
To: git; +Cc: Davi Reis
From: Davi Reis <davi@davi-macbookpro.local>
---
builtin/ls-tree.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index dc86b0d..fa427e2 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
speclen = strlen(spec);
if (speclen <= len)
continue;
+ if (spec[len] != 0 && spec[len] != '/')
+ continue;
if (memcmp(pathname, spec, len))
continue;
return 1;
--
1.7.2.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Added test from David Glasser.
2010-09-09 18:21 [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list davi.reis
@ 2010-09-09 18:21 ` davi.reis
2010-09-09 21:33 ` Matthieu Moy
2010-09-09 21:30 ` [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list Matthieu Moy
1 sibling, 1 reply; 5+ messages in thread
From: davi.reis @ 2010-09-09 18:21 UTC (permalink / raw)
To: git; +Cc: Davi Reis
From: Davi Reis <davi@dhcp-172-19-8-195.mtv.corp.google.com>
---
builtin/ls-tree.c | 2 +-
t/t3100-ls-tree-restrict.sh | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fa427e2..37920d0 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
speclen = strlen(spec);
if (speclen <= len)
continue;
- if (spec[len] != 0 && spec[len] != '/')
+ if (spec[len] != '/')
continue;
if (memcmp(pathname, spec, len))
continue;
diff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh
index eee0d34..9dd74b5 100755
--- a/t/t3100-ls-tree-restrict.sh
+++ b/t/t3100-ls-tree-restrict.sh
@@ -165,4 +165,13 @@ test_expect_success \
EOF
test_output'
+test_expect_success \
+ 'ls-tree with one path a prefix of the other' \
+ 'git ls-tree $tree path2/baz path2/bazbo >current &&
+ make_expected <<\EOF &&
+040000 tree X path2/baz
+120000 blob X path2/bazbo
+EOF
+ test_output'
+
test_done
--
1.7.2.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] Added test from David Glasser.
2010-09-09 18:21 ` [PATCH 2/2] Added test from David Glasser davi.reis
@ 2010-09-09 21:33 ` Matthieu Moy
2010-09-09 21:44 ` Davi Reis
0 siblings, 1 reply; 5+ messages in thread
From: Matthieu Moy @ 2010-09-09 21:33 UTC (permalink / raw)
To: davi.reis; +Cc: git
davi.reis@gmail.com writes:
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
> speclen = strlen(spec);
> if (speclen <= len)
> continue;
> - if (spec[len] != 0 && spec[len] != '/')
> + if (spec[len] != '/')
This change is not the one advertized for in the title. If you didn't
mean it, then
git send-email --annotate
can be your friend, it gives you a last opportunity to check your
patch before it is sent.
If you did mean it, then it should be justified in the commit message.
> --- a/t/t3100-ls-tree-restrict.sh
> +++ b/t/t3100-ls-tree-restrict.sh
> @@ -165,4 +165,13 @@ test_expect_success \
> EOF
> test_output'
>
> +test_expect_success \
> + 'ls-tree with one path a prefix of the other' \
> + 'git ls-tree $tree path2/baz path2/bazbo >current &&
> + make_expected <<\EOF &&
> +040000 tree X path2/baz
> +120000 blob X path2/bazbo
> +EOF
> + test_output'
> +
> test_done
Adding the test can help people to understand what the first patch is
fixing, hence, I'd suggest either squashing both patches, or putting
the test patch first (with a test_expect_failure), and having the
second turn the test_expect_failure into a test_expect_success (hence,
it's obvious reading the patch that it fixes the test).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] Added test from David Glasser.
2010-09-09 21:33 ` Matthieu Moy
@ 2010-09-09 21:44 ` Davi Reis
0 siblings, 0 replies; 5+ messages in thread
From: Davi Reis @ 2010-09-09 21:44 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
I will squash the patches and try again from scratch. Thanks Matthieu.
On Thu, Sep 9, 2010 at 2:33 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> davi.reis@gmail.com writes:
>
>> --- a/builtin/ls-tree.c
>> +++ b/builtin/ls-tree.c
>> @@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
>> speclen = strlen(spec);
>> if (speclen <= len)
>> continue;
>> - if (spec[len] != 0 && spec[len] != '/')
>> + if (spec[len] != '/')
>
> This change is not the one advertized for in the title. If you didn't
> mean it, then
>
> git send-email --annotate
>
> can be your friend, it gives you a last opportunity to check your
> patch before it is sent.
>
> If you did mean it, then it should be justified in the commit message.
>
>> --- a/t/t3100-ls-tree-restrict.sh
>> +++ b/t/t3100-ls-tree-restrict.sh
>> @@ -165,4 +165,13 @@ test_expect_success \
>> EOF
>> test_output'
>>
>> +test_expect_success \
>> + 'ls-tree with one path a prefix of the other' \
>> + 'git ls-tree $tree path2/baz path2/bazbo >current &&
>> + make_expected <<\EOF &&
>> +040000 tree X path2/baz
>> +120000 blob X path2/bazbo
>> +EOF
>> + test_output'
>> +
>> test_done
>
> Adding the test can help people to understand what the first patch is
> fixing, hence, I'd suggest either squashing both patches, or putting
> the test patch first (with a test_expect_failure), and having the
> second turn the test_expect_failure into a test_expect_success (hence,
> it's obvious reading the patch that it fixes the test).
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
>
--
[]s
Davi de Castro Reis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 18:21 [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list davi.reis
2010-09-09 18:21 ` [PATCH 2/2] Added test from David Glasser davi.reis
@ 2010-09-09 21:30 ` Matthieu Moy
1 sibling, 0 replies; 5+ messages in thread
From: Matthieu Moy @ 2010-09-09 21:30 UTC (permalink / raw)
To: davi.reis; +Cc: git
davi.reis@gmail.com writes:
> Subject: Re: [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is give
Don't try to put everything in the subject line. The subject line
should be < 80 characters (fit in a standard terminal), and it's best
if commands like "git log --oneline" & friends can show it without
truncating it, hence, ~50-60 characters is good.
What about
ls-tree: do not recurse when a directory is a prefix of another
<then, a more detailed description, after a blank line>
(I don't know the code of ls-tree, and I don't really understand
either the bug or the fix by reading this. A good commit message help
people to get convinced that your code is good)
> From: Davi Reis <davi@davi-macbookpro.local>
If you use the same identifier for the commit and when you send the
email, you'll get rid of this « From » in the body. I don't think you
want this davi-macbookpro.local to appear publicly indeed.
You don't have a Signed-Off-By:, please read
Documentation/SubmittingPatches to see how to add it and what it
means.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-09 21:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-09 18:21 [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list davi.reis
2010-09-09 18:21 ` [PATCH 2/2] Added test from David Glasser davi.reis
2010-09-09 21:33 ` Matthieu Moy
2010-09-09 21:44 ` Davi Reis
2010-09-09 21:30 ` [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list Matthieu Moy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).