{"thread":{"id":"25057","subject":"[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.","startedAt":"2010-09-09T18:21:53Z","lastAt":"2010-09-09T21:44:21Z","messageCount":5,"participants":["davi.reis@gmail.com","Matthieu Moy","Davi Reis"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"150378","messageId":"1284056514-4935-1-git-send-email-davi.reis@gmail.com","threadId":"25057","inReplyTo":null,"subject":"[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.","fromName":"","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T18:21:53Z","receivedAt":"2010-09-09T18:21:53Z","isPatch":true,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"From: Davi Reis \n\n---\n builtin/ls-tree.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex dc86b0d..fa427e2 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \t\tspeclen = strlen(spec);\n \t\tif (speclen <= len)\n \t\t\tcontinue;\n+\t\tif (spec[len] != 0 && spec[len] != '/')\n+\t\t\tcontinue;\n \t\tif (memcmp(pathname, spec, len))\n \t\t\tcontinue;\n \t\treturn 1;\n-- \n1.7.2.2\n"},{"id":"150379","messageId":"1284056514-4935-2-git-send-email-davi.reis@gmail.com","threadId":"25057","inReplyTo":"1284056514-4935-1-git-send-email-davi.reis@gmail.com","subject":"[PATCH 2/2] Added test from David Glasser.","fromName":"","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T18:21:54Z","receivedAt":"2010-09-09T18:21:54Z","isPatch":true,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"From: Davi Reis \n\n---\n builtin/ls-tree.c | 2 +-\n t/t3100-ls-tree-restrict.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex fa427e2..37920d0 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \t\tspeclen = strlen(spec);\n \t\tif (speclen <= len)\n \t\t\tcontinue;\n-\t\tif (spec[len] != 0 && spec[len] != '/')\n+\t\tif (spec[len] != '/')\n \t\t\tcontinue;\n \t\tif (memcmp(pathname, spec, len))\n \t\t\tcontinue;\ndiff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh\nindex eee0d34..9dd74b5 100755\n--- a/t/t3100-ls-tree-restrict.sh\n+++ b/t/t3100-ls-tree-restrict.sh\n@@ -165,4 +165,13 @@ test_expect_success \\\n EOF\n test_output'\n \n+test_expect_success \\\n+ 'ls-tree with one path a prefix of the other' \\\n+ 'git ls-tree $tree path2/baz path2/bazbo >current &&\n+ make_expected <<\\EOF &&\n+040000 tree X path2/baz\n+120000 blob X path2/bazbo\n+EOF\n+ test_output'\n+\n test_done\n-- \n1.7.2.2\n"},{"id":"150412","messageId":"vpqocc6vc41.fsf@bauges.imag.fr","threadId":"25057","inReplyTo":"1284056514-4935-1-git-send-email-davi.reis@gmail.com","subject":"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.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-09T21:30:22Z","receivedAt":"2010-09-09T21:30:22Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"davi.reis@gmail.com writes:\n\n> Subject: Re: [PATCH 1/2] Do not let lstree output recursively when a directory whose name is a prefix of the others is give\n\nDon't try to put everything in the subject line. The subject line\nshould be < 80 characters (fit in a standard terminal), and it's best\nif commands like \"git log --oneline\" & friends can show it without\ntruncating it, hence, ~50-60 characters is good.\n\nWhat about\n\nls-tree: do not recurse when a directory is a prefix of another\n\n\n\n(I don't know the code of ls-tree, and I don't really understand\neither the bug or the fix by reading this. A good commit message help\npeople to get convinced that your code is good)\n\n> From: Davi Reis \n\nIf you use the same identifier for the commit and when you send the\nemail, you'll get rid of this « From » in the body. I don't think you\nwant this davi-macbookpro.local to appear publicly indeed.\n\nYou don't have a Signed-Off-By:, please read\nDocumentation/SubmittingPatches to see how to add it and what it\nmeans.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"150413","messageId":"vpqk4muvbyt.fsf@bauges.imag.fr","threadId":"25057","inReplyTo":"1284056514-4935-2-git-send-email-davi.reis@gmail.com","subject":"Re: [PATCH 2/2] Added test from David Glasser.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-09T21:33:30Z","receivedAt":"2010-09-09T21:33:30Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"davi.reis@gmail.com writes:\n\n> --- a/builtin/ls-tree.c\n> +++ b/builtin/ls-tree.c\n> @@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n> \t\tspeclen = strlen(spec);\n> \t\tif (speclen <= len)\n> \t\t\tcontinue;\n> -\t\tif (spec[len] != 0 && spec[len] != '/')\n> +\t\tif (spec[len] != '/')\n\nThis change is not the one advertized for in the title. If you didn't\nmean it, then\n\n git send-email --annotate\n\ncan be your friend, it gives you a last opportunity to check your\npatch before it is sent.\n\nIf you did mean it, then it should be justified in the commit message.\n\n> --- a/t/t3100-ls-tree-restrict.sh\n> +++ b/t/t3100-ls-tree-restrict.sh\n> @@ -165,4 +165,13 @@ test_expect_success \\\n> EOF\n> test_output'\n> \n> +test_expect_success \\\n> + 'ls-tree with one path a prefix of the other' \\\n> + 'git ls-tree $tree path2/baz path2/bazbo >current &&\n> + make_expected <<\\EOF &&\n> +040000 tree X path2/baz\n> +120000 blob X path2/bazbo\n> +EOF\n> + test_output'\n> +\n> test_done\n\nAdding the test can help people to understand what the first patch is\nfixing, hence, I'd suggest either squashing both patches, or putting\nthe test patch first (with a test_expect_failure), and having the\nsecond turn the test_expect_failure into a test_expect_success (hence,\nit's obvious reading the patch that it fixes the test).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"150414","messageId":"AANLkTikrvX-BZFTDA3OyrNP7BZN99=Nb4i6=LMaBGX+-@mail.gmail.com","threadId":"25057","inReplyTo":"vpqk4muvbyt.fsf@bauges.imag.fr","subject":"Re: [PATCH 2/2] Added test from David Glasser.","fromName":"Davi Reis","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T21:44:21Z","receivedAt":"2010-09-09T21:44:21Z","isPatch":true,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"I will squash the patches and try again from scratch. Thanks Matthieu.\n\nOn Thu, Sep 9, 2010 at 2:33 PM, Matthieu Moy\n wrote:\n> davi.reis@gmail.com writes:\n>\n>> --- a/builtin/ls-tree.c\n>> +++ b/builtin/ls-tree.c\n>> @@ -52,7 +52,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n>>               speclen = strlen(spec);\n>>               if (speclen <= len)\n>>                       continue;\n>> -             if (spec[len] != 0 && spec[len] != '/')\n>> +             if (spec[len] != '/')\n>\n> This change is not the one advertized for in the title. If you didn't\n> mean it, then\n>\n>  git send-email --annotate\n>\n> can be your friend, it gives you a last opportunity to check your\n> patch before it is sent.\n>\n> If you did mean it, then it should be justified in the commit message.\n>\n>> --- a/t/t3100-ls-tree-restrict.sh\n>> +++ b/t/t3100-ls-tree-restrict.sh\n>> @@ -165,4 +165,13 @@ test_expect_success \\\n>>  EOF\n>>       test_output'\n>>\n>> +test_expect_success \\\n>> +    'ls-tree with one path a prefix of the other' \\\n>> +    'git ls-tree $tree path2/baz path2/bazbo >current &&\n>> +     make_expected <<\\EOF &&\n>> +040000 tree X  path2/baz\n>> +120000 blob X  path2/bazbo\n>> +EOF\n>> +     test_output'\n>> +\n>>  test_done\n>\n> Adding the test can help people to understand what the first patch is\n> fixing, hence, I'd suggest either squashing both patches, or putting\n> the test patch first (with a test_expect_failure), and having the\n> second turn the test_expect_failure into a test_expect_success (hence,\n> it's obvious reading the patch that it fixes the test).\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n>\n\n\n\n-- \n[]s\nDavi de Castro Reis\n"}]}