{"thread":{"id":"22415","subject":"[PATCH] Fix git rev-list --reverse --max-count=N","startedAt":"2010-01-27T20:03:20Z","lastAt":"2010-01-27T22:51:02Z","messageCount":6,"participants":["Michael Spang","Johannes Sixt","Sverre Rabbelier","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"132831","messageId":"1264622600-20981-1-git-send-email-spang@google.com","threadId":"22415","inReplyTo":null,"subject":"[PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Michael Spang","fromEmail":"spang@google.com","sentAt":"2010-01-27T20:03:20Z","receivedAt":"2010-01-27T20:03:20Z","isPatch":true,"sender":{"key":"spang@google.com","avatar":null},"body":"Using --max-count with --reverse currently outputs the last N commits\nin the final output rather than the first N commits. We want to\ntruncate the reversed list after the first few commits, rather than\ntruncating the initial list and reversing that.\n\nSigned-off-by: Michael Spang \n---\n revision.c | 20 +++++++++++++++++---\n 1 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex f54d43f..62135e0 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1993,7 +1993,8 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \t\tc = NULL;\n \t\tbreak;\n \tdefault:\n-\t\trevs->max_count--;\n+\t\tif (!revs->reverse)\n+\t\t\trevs->max_count--;\n \t}\n \n \tif (c)\n@@ -2055,8 +2056,21 @@ struct commit *get_revision(struct rev_info *revs)\n \t\trevs->reverse_output_stage = 1;\n \t}\n \n-\tif (revs->reverse_output_stage)\n-\t\treturn pop_commit(&revs->commits);\n+\tif (revs->reverse_output_stage) {\n+\t\tc = pop_commit(&revs->commits);\n+\n+\t\tswitch (revs->max_count) {\n+\t\tcase -1:\n+\t\t\tbreak;\n+\t\tcase 0:\n+\t\t\tc = NULL;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\trevs->max_count--;\n+\t\t}\n+\n+\t\treturn c;\n+\t}\n \n \tc = get_revision_internal(revs);\n \tif (c && revs->graph)\n-- \n1.6.6\n"},{"id":"132838","messageId":"201001272309.26054.j6t@kdbg.org","threadId":"22415","inReplyTo":"1264622600-20981-1-git-send-email-spang@google.com","subject":"Re: [PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-01-27T22:09:25Z","receivedAt":"2010-01-27T22:09:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Mittwoch, 27. Januar 2010, Michael Spang wrote:\n> Using --max-count with --reverse currently outputs the last N commits\n> in the final output rather than the first N commits. We want to\n> truncate the reversed list after the first few commits, rather than\n> truncating the initial list and reversing that.\n\nSo when you have this history (A is oldest, D is newest):\n\n A--B--C--D\n\nand you say\n\n git log --max-count=2 --reverse D\n\nthen you want\n\n A\n B\n\nbut I want\n\n C\n D\n\nWhy is your interpretation correct, an mine wrong?\n\n-- Hannes\n"},{"id":"132839","messageId":"fabb9a1e1001271417w1c26fffdy379f8b845e4098b1@mail.gmail.com","threadId":"22415","inReplyTo":"201001272309.26054.j6t@kdbg.org","subject":"Re: [PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-01-27T22:17:11Z","receivedAt":"2010-01-27T22:17:11Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\n> So when you have this history (A is oldest, D is newest):\n>\n>   A--B--C--D\n>\n> and you say\n>\n>   git log --max-count=2 --reverse D\n>\n> then you want\n>\n>   A\n>   B\n>\n> but I want\n>\n>   C\n>   D\n>\n\nAnd the current behavior is\n\nB\nA\n\nIsn't it? I agree btw, that C D is the 'correct' result.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"132841","messageId":"abef960f1001271421s6c0f72f7xd2697ab34b53d1ea@mail.gmail.com","threadId":"22415","inReplyTo":"201001272309.26054.j6t@kdbg.org","subject":"Re: [PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Michael Spang","fromEmail":"spang@google.com","sentAt":"2010-01-27T22:21:47Z","receivedAt":"2010-01-27T22:21:47Z","isPatch":true,"sender":{"key":"spang@google.com","avatar":null},"body":"On Wed, Jan 27, 2010 at 5:09 PM, Johannes Sixt wrote:\n> On Mittwoch, 27. Januar 2010, Michael Spang wrote:\n>\n> So when you have this history (A is oldest, D is newest):\n>\n>   A--B--C--D\n>\n> and you say\n>\n>   git log --max-count=2 --reverse D\n>\n> then you want\n>\n>   A\n>   B\n>\n> but I want\n>\n>   C\n>   D\n>\n> Why is your interpretation correct, an mine wrong?\n\nPerhaps not wrong, but for me it was unexpected. For whatever reason,\nI expected \"--reverse\" to give you the illusion that you are iterating\nfrom the beginning of the history, even if it's not actually possible\nto iterate that way directly. In line with that, limiting the output\nto N commits would give you the earliest N commits. If you instead\nthink \"stop descending through the history once we have N commits\" the\ncurrent behavior makes sense.\n\nI talked to Junio and he says the current behavior is here to stay.\n\nMichael\n"},{"id":"132842","messageId":"7vsk9rnpxo.fsf@alter.siamese.dyndns.org","threadId":"22415","inReplyTo":"201001272309.26054.j6t@kdbg.org","subject":"Re: [PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-27T22:28:03Z","receivedAt":"2010-01-27T22:28:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> On Mittwoch, 27. Januar 2010, Michael Spang wrote:\n>> Using --max-count with --reverse currently outputs the last N commits\n>> in the final output rather than the first N commits. We want to\n>> truncate the reversed list after the first few commits, rather than\n>> truncating the initial list and reversing that.\n>\n> So when you have this history (A is oldest, D is newest):\n>\n> A--B--C--D\n>\n> and you say\n>\n> git log --max-count=2 --reverse D\n>\n> then you want\n>\n> A\n> B\n>\n> but I want\n>\n> C\n> D\n>\n> Why is your interpretation correct, an mine wrong?\n\nThe interaction between --max-count and --reverse was designed a long time\nago to support \"I want to review the recent N commits in order to make\nsure that they are natural and logical progression\". So an unconditional\nchange of semantics to break that expectation this late in the game will\nnever be acceptable, and giving title to such a patch with a word \"Fix\"\nwon't fly well (it simply _breaks_ behaviour users have long learned to\nexpect).\n\nIt is a different matter to add an explicit command line option (and no\nconfiguration to change it to default) to apply reversal before count\nlimiting.\n"},{"id":"132844","messageId":"abef960f1001271451n5ed454aaye1591ea35729b53f@mail.gmail.com","threadId":"22415","inReplyTo":"7vsk9rnpxo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix git rev-list --reverse --max-count=N","fromName":"Michael Spang","fromEmail":"spang@google.com","sentAt":"2010-01-27T22:51:02Z","receivedAt":"2010-01-27T22:51:02Z","isPatch":true,"sender":{"key":"spang@google.com","avatar":null},"body":"On Wed, Jan 27, 2010 at 5:28 PM, Junio C Hamano wrote:\n> Johannes Sixt writes:\n>\n> The interaction between --max-count and --reverse was designed a long time\n> ago to support \"I want to review the recent N commits in order to make\n> sure that they are natural and logical progression\".  So an unconditional\n> change of semantics to break that expectation this late in the game will\n> never be acceptable, and giving title to such a patch with a word \"Fix\"\n> won't fly well (it simply _breaks_ behaviour users have long learned to\n> expect).\n\nYou have my apology for calling it a fix. I just don't have the same\nvisibility into what undocumented behavior users depend on that you\ndo, and so I thought it actually was one.\n\nThe patch can die here. I understand the current behavior now, and I\ndon't have a strong need for the behavior I expected (piping to head\nworks fine).\n\nMichael\n"}]}