{"thread":{"id":"11401","subject":"[PATCH] Fix \"git log --diff-filter\" bug","startedAt":"2007-12-25T11:06:47Z","lastAt":"2008-01-07T01:58:22Z","messageCount":9,"participants":["Arjen Laarhoven","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"64092","messageId":"1198580807-18802-1-git-send-email-arjen@yaph.org","threadId":"11401","inReplyTo":null,"subject":"[PATCH] Fix \"git log --diff-filter\" bug","fromName":"Arjen Laarhoven","fromEmail":"arjen@yaph.org","sentAt":"2007-12-25T11:06:47Z","receivedAt":"2007-12-25T11:06:47Z","isPatch":true,"sender":{"key":"arjen@yaph.org","avatar":"https://gravatar.com/avatar/f776c2c0c5ea62d70827b942eb7d95ce85661a3d70bc3f03cf9773815c599c01?d=mp&s=160"},"body":"In commit b7bb760d5ed4881422673d32f869d140221d3564 an optimization\nwas made to avoid unnecessary diff generation.  This was partly fixed\nin 99516e35d096f41e7133cacde8fbed8ee9a3ecd0, but obviously the\n'--diff-filter' option also needs the diff machinery in action.\n\nSigned-off-by: Arjen Laarhoven <arjen@yaph.org>\n---\n revision.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e2f4f1..6e85aaa 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1290,8 +1290,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \tif (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)\n \t\trevs->diff = 1;\n \n-\t/* Pickaxe and rename following needs diffs */\n-\tif (revs->diffopt.pickaxe || DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n+\t/* Pickaxe, diff-filter and rename following need diffs */\n+\tif (revs->diffopt.pickaxe ||\n+\t    revs->diffopt.filter ||\n+\t    DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n \t\trevs->diff = 1;\n \n \tif (revs->topo_order)\n-- \n1.5.4.rc1.21.g0e545\n"},{"id":"64098","messageId":"m3r6hao0nu.fsf@roke.D-201","threadId":"11401","inReplyTo":"1198580807-18802-1-git-send-email-arjen@yaph.org","subject":"Re: [PATCH] Fix \"git log --diff-filter\" bug","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-12-25T22:44:29Z","receivedAt":"2007-12-25T22:44:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Arjen Laarhoven <arjen@yaph.org> writes:\n\n> In commit b7bb760d5ed4881422673d32f869d140221d3564 an optimization\n> was made to avoid unnecessary diff generation.  This was partly fixed\n> in 99516e35d096f41e7133cacde8fbed8ee9a3ecd0, but obviously the\n> '--diff-filter' option also needs the diff machinery in action.\n\nThanks a lot! I was wondering why 'git log --diff-filter=M' didn't\nfind anything...\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"64105","messageId":"7vve6lmegc.fsf@gitster.siamese.dyndns.org","threadId":"11401","inReplyTo":"1198580807-18802-1-git-send-email-arjen@yaph.org","subject":"Re: [PATCH] Fix \"git log --diff-filter\" bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-26T19:41:07Z","receivedAt":"2007-12-26T19:41:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.  Some tests?\n"},{"id":"64547","messageId":"1199571622-12953-1-git-send-email-jnareb@gmail.com","threadId":"11401","inReplyTo":"1198580807-18802-1-git-send-email-arjen@yaph.org","subject":"[PATCH] Test \"git log --diff-filter\"","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-01-05T22:20:22Z","receivedAt":"2008-01-05T22:20:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add test to check \"git log --diff-filter\" works correctly with and\nwithout diff generation by git-log; the main purpose of this test is\nto check if \"git log --diff-filter\" filters revisions correctly.\n\nThis is a companion test to commit 0faf2da7e5ee5c2f472d8a7afaf8616101f34e80\n(Fix \"git log --diff-filter\" bug) by Arjen Laarhoven.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nJunio C Hamano wrote:\n\n> Thanks.  Some tests?\n\nSo there it is...\n\n\nFirst, either I don't understand what referred to commit was supposed\nto fix, or my test is wrong, or the patch doesn't fix the bug.\n\nSecond, I have a few questions about the test itself. I'm not that\nsure about it's name: t/README tells us to use 4 as a first digit of\ntest number for testing diff commands, and 8 for commands concerning\nforensics. \"git log --diff-filter\" is a forensics concerning diff\noutput. Second digit is for command itself: 0 is used for diff, 2 for\nlog, so perhaps the test should be named t/t4203-log-diff-filter.sh\n\nThe style of writing test is not very consistent across git test\nsuite. I think that the 'setup' step style is all right, and only\nperhas the style of those two-liner tests could be changed.\n\nI use \"git diff --exit-code expected current\" instead of \"diff\" or\n\"cmp\" utilities; should all (new) test use this... well of course\nexcept ones testing diff output itself?\n\n t/t4025-diff-filter.sh |  240 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 240 insertions(+), 0 deletions(-)\n create mode 100755 t/t4025-diff-filter.sh\n\ndiff --git a/t/t4025-diff-filter.sh b/t/t4025-diff-filter.sh\nnew file mode 100755\nindex 0000000..3113786\n--- /dev/null\n+++ b/t/t4025-diff-filter.sh\n@@ -0,0 +1,240 @@\n+#!/bin/sh\n+\n+test_description='git log --diff-filter option\n+\n+Test --diff-filter option with git-log with and without diff output,\n+checking both diff output filtering and revision list filtering.\n+'\n+\n+. ./test-lib.sh\n+. ../diff-lib.sh ;# test-lib chdir's into trash\n+\n+# ----------------------------------------------------------------------\n+\n+test_expect_success setup '\n+\n+\trm -f foo &&\n+\tcat ../../COPYING >foo &&\n+\tgit add foo &&\n+\tgit commit -a -m \"1st commit: A\" &&\n+\n+\tcp foo bar &&\n+\tgit add bar &&\n+\tgit commit -a -m \"2nd commit: C\" &&\n+\n+\tgit rm foo &&\n+\tgit commit -a -m \"3rd commit: D\" &&\n+\n+\techo \"First added line\" >> bar &&\n+\tgit commit -a -m \"4th commit: M\" &&\n+\n+\tgit mv bar foo &&\n+\tgit commit -a -m \"5th commit: R\" &&\n+\n+\trm -f foo &&\n+\tcat ../../Makefile >foo &&\n+\tgit commit -a -m \"6th commit: B\" &&\n+\n+\trm -f bar &&\n+\techo \"bar\" > bar &&\n+\tgit add bar &&\n+\techo \"Second added line\" >> foo &&\n+\tgit commit -a -m \"7th commit: AM\"\n+\n+\trm -f bar &&\n+\tln -s foo bar &&\n+\tgit commit -a -m \"8th commit: T\"\n+\n+'\n+\n+# ----------------------------------------------------------------------\n+\n+cat >expected <<\\EOF\n+7th commit: AM\n+:000000 100644 0000000000000000000000000000000000000000 5716ca5987cbf97d6bb54920bea6adde242d87e6 A\tbar\n+\n+1st commit: A\n+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 A\tfoo\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=A' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=A >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+3rd commit: D\n+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D\tfoo\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=D' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=D >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+7th commit: AM\n+:100644 100644 21c80e6bf73163b9770cba5331cd48172fa6d43e a892bacce2a80efc14eef1c316e827575a96e5c9 M\tfoo\n+\n+4th commit: M\n+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 915b225a6c9984e645a8061e05002f8cbd2ce46c M\tbar\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=M' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=M >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+5th commit: R\n+:100644 100644 915b225a6c9984e645a8061e05002f8cbd2ce46c 915b225a6c9984e645a8061e05002f8cbd2ce46c R100\tbar\tfoo\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=R' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=R >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+2nd commit: C\n+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100\tfoo\tbar\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=C' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=C >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+6th commit: B\n+:100644 100644 915b225a6c9984e645a8061e05002f8cbd2ce46c 21c80e6bf73163b9770cba5331cd48172fa6d43e M098\tfoo\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=B' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=B >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+8th commit: T\n+:100644 120000 5716ca5987cbf97d6bb54920bea6adde242d87e6 19102815663d23f8b75a47e7a01965dcdc96468c T\tbar\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=T' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=T >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+cat >expected <<\\EOF\n+7th commit: AM\n+:000000 100644 0000000000000000000000000000000000000000 5716ca5987cbf97d6bb54920bea6adde242d87e6 A\tbar\n+:100644 100644 21c80e6bf73163b9770cba5331cd48172fa6d43e a892bacce2a80efc14eef1c316e827575a96e5c9 M\tfoo\n+\n+1st commit: A\n+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 A\tfoo\n+EOF\n+\n+test_expect_success 'git log --raw --diff-filter=A*' '\n+\tgit log --raw --no-abbrev --pretty=format:%s -B -C -C --diff-filter=A* >current &&\n+\tcompare_diff_raw expected current\n+'\n+\n+# ----------------------------------------------------------------------\n+\n+cat >expected <<\\EOF\n+8th commit: T\n+7th commit: AM\n+6th commit: B\n+5th commit: R\n+4th commit: M\n+3rd commit: D\n+2nd commit: C\n+1st commit: A\n+EOF\n+\n+test_expect_success 'git log (no filter)' '\n+\tgit log --pretty=format:%s -B -C -C >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+7th commit: AM\n+1st commit: A\n+EOF\n+\n+test_expect_success 'git log --diff-filter=A' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=A >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+3rd commit: D\n+EOF\n+\n+test_expect_success 'git log --diff-filter=D' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=D >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+7th commit: AM\n+4th commit: M\n+EOF\n+\n+test_expect_success 'git log --diff-filter=M' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=M >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+5th commit: R\n+EOF\n+\n+test_expect_success 'git log --diff-filter=R' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=R >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+2nd commit: C\n+EOF\n+\n+test_expect_success 'git log --diff-filter=C' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=C >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+6th commit: B\n+EOF\n+\n+test_expect_success 'git log --diff-filter=B' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=A >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+\n+cat >expected <<\\EOF\n+8th commit: T\n+EOF\n+\n+test_expect_success 'git log --diff-filter=T' '\n+\tgit log --pretty=format:%s -B -C -C --diff-filter=T >current &&\n+\tgit diff --exit-code expected current\n+'\n+\n+# ----------------------------------------------------------------------\n+\n+test_done\n-- \n1.5.3.7\n"},{"id":"64550","messageId":"7vsl1b7vhb.fsf@gitster.siamese.dyndns.org","threadId":"11401","inReplyTo":"1199571622-12953-1-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH] Test \"git log --diff-filter\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-05T22:34:08Z","receivedAt":"2008-01-05T22:34:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Add test to check \"git log --diff-filter\" works correctly with and\n> without diff generation by git-log; the main purpose of this test is\n> to check if \"git log --diff-filter\" filters revisions correctly.\n>\n> This is a companion test to commit 0faf2da7e5ee5c2f472d8a7afaf8616101f34e80\n> (Fix \"git log --diff-filter\" bug) by Arjen Laarhoven.\n\nIf you look at the commit, you'd notice that I've added\nnecessary test when I accepted the patch from Arjen already ;-).\n\nDoes this new set of tests check something new?\n"},{"id":"64554","messageId":"200801060033.03672.jnareb@gmail.com","threadId":"11401","inReplyTo":"7vsl1b7vhb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Test \"git log --diff-filter\"","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-01-05T23:33:02Z","receivedAt":"2008-01-05T23:33:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Add test to check \"git log --diff-filter\" works correctly with and\n> > without diff generation by git-log; the main purpose of this test is\n> > to check if \"git log --diff-filter\" filters revisions correctly.\n> >\n> > This is a companion test to commit 0faf2da7e5ee5c2f472d8a7afaf8616101f34e80\n> > (Fix \"git log --diff-filter\" bug) by Arjen Laarhoven.\n> \n> If you look at the commit, you'd notice that I've added\n> necessary test when I accepted the patch from Arjen already ;-).\n\nSorry for the noise, then.\n\n> Does this new set of tests check something new?\n\nMy test checks all --diff-filter filters relevant to git-diff-tree,\ni.e. ADMRCBT, and not only AMD.\n\nAlso it checks if the diff is shown correctly for --diff-filter=M and\nfor --diff-filter=M*, but I think this should be a separate test, and\nuse only git-diff-something, and not git-log.\n\n\nP.S. By the way, it is IMHO a bit strange that --pretty=oneline uses\nnewline as a terminator (it means that there is a newline at the end of\n\"git log --pretty=oneline), while --pretty=\"format:%s\" uses newline as\na separator (meaning that there is no newline at the end) when redirected\nto file.\n\n # git log --pretty=\"format:%s\" -B -C -C >current\n\nThe 'current' file doesn't end with newline (with --pretty=oneline it\ndoes), when log ends at root commit. Strange.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"64562","messageId":"7vmyrj7kq5.fsf@gitster.siamese.dyndns.org","threadId":"11401","inReplyTo":"200801060033.03672.jnareb@gmail.com","subject":"Re: [PATCH] Test \"git log --diff-filter\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-06T02:26:26Z","receivedAt":"2008-01-06T02:26:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> My test checks all --diff-filter filters relevant to git-diff-tree,\n> i.e. ADMRCBT, and not only AMD.\n\nAh, I see.  Thanks --- that could have been stated in the log\nmessage.  Maybe we would want to add them to existing test\nscript, instead of adding a whole new one?\n\n> P.S. By the way, it is IMHO a bit strange that --pretty=oneline uses\n> newline as a terminator (it means that there is a newline at the end of\n> \"git log --pretty=oneline), while --pretty=\"format:%s\" uses newline as\n> a separator...\n\nYeah, I tend to agree, although I learned to live with it long\ntime ago.\n"},{"id":"64610","messageId":"200801070131.57722.jnareb@gmail.com","threadId":"11401","inReplyTo":"7vmyrj7kq5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Test \"git log --diff-filter\"","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-01-07T00:31:56Z","receivedAt":"2008-01-07T00:31:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > My test checks all --diff-filter filters relevant to git-diff-tree,\n> > i.e. ADMRCBT, and not only AMD.\n> \n> Ah, I see.  Thanks --- that could have been stated in the log\n> message.  Maybe we would want to add them to existing test\n> script, instead of adding a whole new one?\n\nThe test as it stands now checks if --diff-filter select appropriate\nrevisions, even without patch output. I think it is enough, as I don't\nsee how we could screw up to filter AMD correctly, and not all others...\n...perhaps with exception of pair breaking, and how they are filtered\nusing --diff-filter=M and --diff-filter=B; but this impression might\nbe caused by the fact that pair breaking is the only one which doesn't\nuse symbol ('B') in raw diff format output.\n\n> > P.S. By the way, it is IMHO a bit strange that --pretty=oneline uses\n> > newline as a terminator (it means that there is a newline at the end of\n> > \"git log --pretty=oneline), while --pretty=\"format:%s\" uses newline as\n> > a separator...\n> \n> Yeah, I tend to agree, although I learned to live with it long\n> time ago.\n\nIMHO that is design bug. Perhaps it should be changed? This way, at least\nconceptually oneline, short, medium, full, fuller, email formats might be\nconsidered simply pre-defined format:<sth> formats.\n\nAm I mistaken in thinking that the rest of git always use terminators,\nand not separators for records output?\n-- \nJakub Narebski\nPoland\n"},{"id":"64613","messageId":"7vabni2y81.fsf@gitster.siamese.dyndns.org","threadId":"11401","inReplyTo":"200801070131.57722.jnareb@gmail.com","subject":"Re: [PATCH] Test \"git log --diff-filter\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-07T01:58:22Z","receivedAt":"2008-01-07T01:58:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Am I mistaken in thinking that the rest of git always use terminators,\n> and not separators for records output?\n\nActually, for normal \"git log\", separator semantics is the right\nthing to use.  You do not want to add an additional trailing\nnewline when you show only one.  Compare these two to see what I\nmean:\n\n\t$ git log -1\n        $ git log -2\n\nThe problem is with --pretty=format.\n\n        \n"}]}