{"thread":{"id":"24821","subject":"[PATCH 0/2] log/ format-patch improvements","startedAt":"2010-08-21T20:28:49Z","lastAt":"2010-08-26T17:52:38Z","messageCount":11,"participants":["Ramkumar Ramachandra","Jonathan Nieder","Junio C Hamano","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"148683","messageId":"1282422531-29673-1-git-send-email-artagnon@gmail.com","threadId":"24821","inReplyTo":null,"subject":"[PATCH 0/2] log/ format-patch improvements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T20:28:49Z","receivedAt":"2010-08-21T20:28:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThe first patch implements Jakub's suggestion. Arguably, it's slightly\ncomplicated- it took me more than a few minutes to do the math with\n`nr` and `nr_i`.\n\nThe second patch clarifies the meaning of the `-` option. We should\nalso probably force the mutual exclusivity of `-` and to avoid confusion.\n\nAdditionally, thanks to Thomas for drilling into me the fundamental\ndifference between - and a revision range (on IRC).\n\nRamkumar Ramachandra (2):\n git-format-patch: Print a diagnostic message when ignoring commits\n log: Improve description of '-' option in documentation\n\n Documentation/git-format-patch.txt | 2 +-\n Documentation/git-log.txt | 2 +-\n builtin/log.c | 42 ++++++++++++++++++++++++++---------\n 3 files changed, 33 insertions(+), 13 deletions(-)\n\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148685","messageId":"1282422531-29673-2-git-send-email-artagnon@gmail.com","threadId":"24821","inReplyTo":"1282422531-29673-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T20:28:50Z","receivedAt":"2010-08-21T20:28:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Earlier, git-format-patch used to silently skip over commits that it\ndidn't intend to make patches out of. As a consequence, a command like\n'git-format-patch -3' would just do nothing and print nothing if the\ntopmost three commits were merge commits. Instead, print a useful\nmessage similar to \"Skipping: Merge branch ...\" when ignoring a\ncommit.\n\nSuggested-by: Jakub Narebski \nCc: Thomas Rast \nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/log.c | 42 +++++++++++++++++++++++++++++++-----------\n 1 files changed, 31 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0151d2f..b64de7c 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1090,7 +1090,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct commit **list = NULL;\n \tstruct rev_info rev;\n \tstruct setup_revision_opt s_r_opt;\n-\tint nr = 0, total, i;\n+\tint nr = 0, nr_i = 0, total, i;\n \tint use_stdout = 0;\n \tint start_number = -1;\n \tint numbered_files = 0;\t\t/* _just_ numbers */\n@@ -1098,6 +1098,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint cover_letter = 0;\n \tint boundary_count = 0;\n \tint no_binary_diff = 0;\n+\tint *list_i = NULL;\n \tstruct commit *origin = NULL, *head = NULL;\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n@@ -1342,19 +1343,22 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \n-\t\t/* ignore merges */\n-\t\tif (commit->parents && commit->parents->next)\n-\t\t\tcontinue;\n-\n-\t\tif (ignore_if_in_upstream &&\n-\t\t\t\thas_commit_patch_id(commit, &ids))\n-\t\t\tcontinue;\n+\t\t/* ignore merge commits and optionally ignore commits\n+\t\t already in upstream */\n+\t\tif ((commit->parents && commit->parents->next) ||\n+\t\t (ignore_if_in_upstream &&\n+\t\t has_commit_patch_id(commit, &ids))) {\n+\t\t\t/* Store the nr of the ignored commits in list_i */\n+\t\t\tnr_i++;\n+\t\t\tlist_i = xrealloc(list_i, nr_i * sizeof(list_i[0]));\n+\t\t\tlist_i[nr_i - 1] = nr;\n+\t\t}\n \n \t\tnr++;\n \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n \t\tlist[nr - 1] = commit;\n \t}\n-\ttotal = nr;\n+\ttotal = nr - nr_i;\n \tif (!keep_subject && auto_number && total > 1)\n \t\tnumbered = 1;\n \tif (numbered)\n@@ -1376,10 +1380,25 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tstart_number--;\n \t}\n \trev.add_signoff = add_signoff;\n-\twhile (0 <= --nr) {\n+\tfor (i = nr - nr_i; --nr >= 0;) {\n \t\tint shown;\n \t\tcommit = list[nr];\n-\t\trev.nr = total - nr + (start_number - 1);\n+\n+\t\t/* Ignore commits in list whose index is list_i */\n+\t\tif (list_i[nr_i - 1] == nr) {\n+\t\t\tstruct strbuf commit_msg = STRBUF_INIT;\n+\t\t\tstruct pretty_print_context ctx = {0};\n+\t\t\tformat_commit_message(commit, \"%s\", &commit_msg, &ctx);\n+\t\t\tfprintf(realstdout, \"Skipping: %s\\n\",\n+\t\t\t\tcommit_msg.buf);\n+\t\t\tstrbuf_release(&buf);\n+\t\t\t--nr_i;\n+\t\t\tcontinue;\n+\t\t}\n+\t\telse\n+\t\t\t--i;\n+\n+\t\trev.nr = total - i + (start_number - 1);\n \t\t/* Make the second and subsequent mails replies to the first */\n \t\tif (thread) {\n \t\t\t/* Have we already had a message ID? */\n@@ -1443,6 +1462,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tfclose(stdout);\n \t}\n \tfree(list);\n+\tfree(list_i);\n \tstring_list_clear(&extra_to, 0);\n \tstring_list_clear(&extra_cc, 0);\n \tstring_list_clear(&extra_hdr, 0);\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148684","messageId":"1282422531-29673-3-git-send-email-artagnon@gmail.com","threadId":"24821","inReplyTo":"1282422531-29673-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] log: Improve description of '-' option in documentation","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-21T20:28:51Z","receivedAt":"2010-08-21T20:28:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The earlier description of the '-' option was misleading- the user\nwould have expected to be able to use it to limit the number of\ncommits shown when specifying a revision range, for example. In\nreality, the option simply instructs the log to walk the topmost \ncommits. Also update the meaning of the same option in the\ngit-format-patch documentation.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Documentation/git-format-patch.txt | 2 +-\n Documentation/git-log.txt | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 4b3f5ba..df77474 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -74,7 +74,7 @@ OPTIONS\n include::diff-options.txt[]\n \n -::\n-\tLimits the number of patches to prepare.\n+\tPrepare patches from the topmost commits.\n \n -o ::\n --output-directory ::\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 83e4ee3..ca02206 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -28,7 +28,7 @@ OPTIONS\n -------\n \n -::\n-\tLimits the number of commits to show.\n+\tShow the topmost commits.\n \n ..::\n \tShow only commits between the named two commits. When\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"148959","messageId":"20100825084416.GC3280@kytes","threadId":"24821","inReplyTo":"1282422531-29673-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-25T08:44:21Z","receivedAt":"2010-08-25T08:44:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nRamkumar Ramachandra writes:\n> The first patch implements Jakub's suggestion. Arguably, it's slightly\n> complicated- it took me more than a few minutes to do the math with\n> `nr` and `nr_i`.\n> \n> The second patch clarifies the meaning of the `-` option. We should\n> also probably force the mutual exclusivity of `-` and range> to avoid confusion.\n> \n> Additionally, thanks to Thomas for drilling into me the fundamental\n> difference between - and a revision range (on IRC).\n> \n> Ramkumar Ramachandra (2):\n> git-format-patch: Print a diagnostic message when ignoring commits\n> log: Improve description of '-' option in documentation\n> \n> Documentation/git-format-patch.txt | 2 +-\n> Documentation/git-log.txt | 2 +-\n> builtin/log.c | 42 ++++++++++++++++++++++++++---------\n> 3 files changed, 33 insertions(+), 13 deletions(-)\n\nDo you see value in this patch or is it just unnecessary baggage?\n\n-- Ram\n"},{"id":"148996","messageId":"20100825205416.GB2319@burratino","threadId":"24821","inReplyTo":"20100825084416.GC3280@kytes","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-25T20:54:16Z","receivedAt":"2010-08-25T20:54:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Ramkumar Ramachandra writes:\n\n>> The second patch clarifies the meaning of the `-` option. We should\n>> also probably force the mutual exclusivity of `-` and > range> to avoid confusion.\n[...]\n> Do you see value in this patch or is it just unnecessary baggage?\n\nI see value in avoiding confusion. Maybe one solution would be to make\nformat-patch use --no-merges by default.\n\n $ git log --oneline --no-merges -3 ab/test..origin/pu\n 70256a3 shell: Rewrite documentation and improve error message\n 9c46c05 rev-parse: tests git rev-parse --verify master@{n}, for various n\n eedce78 sha1_name.c: use warning in preference to fprintf(stderr\n"},{"id":"149008","messageId":"7vpqx6pcok.fsf@alter.siamese.dyndns.org","threadId":"24821","inReplyTo":"20100825084416.GC3280@kytes","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-25T22:09:15Z","receivedAt":"2010-08-25T22:09:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Ramkumar Ramachandra writes:\n>> The first patch implements Jakub's suggestion. Arguably, it's slightly\n>> complicated- it took me more than a few minutes to do the math with\n>> `nr` and `nr_i`.\n>> \n>> The second patch clarifies the meaning of the `-` option. We should\n>> also probably force the mutual exclusivity of `-` and > range> to avoid confusion.\n>> \n>> Additionally, thanks to Thomas for drilling into me the fundamental\n>> difference between - and a revision range (on IRC).\n>> \n>> Ramkumar Ramachandra (2):\n>> git-format-patch: Print a diagnostic message when ignoring commits\n>> log: Improve description of '-' option in documentation\n>> \n>> Documentation/git-format-patch.txt | 2 +-\n>> Documentation/git-log.txt | 2 +-\n>> builtin/log.c | 42 ++++++++++++++++++++++++++---------\n>> 3 files changed, 33 insertions(+), 13 deletions(-)\n>\n> Do you see value in this patch or is it just unnecessary baggage?\n\nI am not very impressed by the counting. It probably makes more sense to\ncount only what we are actually going to process and emit, i.e. always use\nno-merges (do we even support format-patch on a merge?). \n"},{"id":"149033","messageId":"20100826053439.GB9407@kytes","threadId":"24821","inReplyTo":"20100825205416.GB2319@burratino","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-26T05:34:42Z","receivedAt":"2010-08-26T05:34:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan and Junio,\n\nJunio C Hamano writes:\n> I am not very impressed by the counting. It probably makes more sense to\n> count only what we are actually going to process and emit, i.e. always use\n> no-merges (do we even support format-patch on a merge?). \n\nFrankly, I think the patch looks like an ugly hack myself. No,\nformat-patch doesn't support merge commits at all.\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n> > Ramkumar Ramachandra writes:\n> \n> >> The second patch clarifies the meaning of the `-` option. We should\n> >> also probably force the mutual exclusivity of `-` and >> range> to avoid confusion.\n> [...]\n> > Do you see value in this patch or is it just unnecessary baggage?\n> \n> I see value in avoiding confusion. Maybe one solution would be to make\n> format-patch use --no-merges by default.\n\nGood idea. I'll write a patch. Do we also want people to be able to\nturn off `--no-merges`? If so, how?\n\n-- Ram\n"},{"id":"149035","messageId":"20100826054659.GC9708@burratino","threadId":"24821","inReplyTo":"20100826053439.GB9407@kytes","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-26T05:46:59Z","receivedAt":"2010-08-26T05:46:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> No,\n> format-patch doesn't support merge commits at all.\n[...]\n\n> Jonathan Nieder writes:\n>\n>> I see value in avoiding confusion. Maybe one solution would be to make\n>> format-patch use --no-merges by default.\n>\n> Good idea. I'll write a patch. Do we also want people to be able to\n> turn off `--no-merges`?\n\nI don't see a need for it.\n\nHowever, if you can think of good names for --undo-no-merges and\n--undo-merges options to \"git log\", that might be a nice independent\nchange for the revision option parser.\n"},{"id":"149050","messageId":"201008260906.20497.trast@student.ethz.ch","threadId":"24821","inReplyTo":"20100826054659.GC9708@burratino","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-08-26T07:06:20Z","receivedAt":"2010-08-26T07:06:20Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jonathan Nieder wrote:\n> However, if you can think of good names for --undo-no-merges and\n> --undo-merges options to \"git log\", that might be a nice independent\n> change for the revision option parser.\n\n--merges={include,only,exclude} or so, and then have --merges be\n--merges=only and --no-merges be --merges=exclude?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"149066","messageId":"7vk4ndo05p.fsf@alter.siamese.dyndns.org","threadId":"24821","inReplyTo":"20100826054659.GC9708@burratino","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-26T15:37:22Z","receivedAt":"2010-08-26T15:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Ramkumar Ramachandra wrote:\n> ...\n>> Good idea. I'll write a patch. Do we also want people to be able to\n>> turn off `--no-merges`?\n>\n> I don't see a need for it.\n>\n> However, if you can think of good names for --undo-no-merges and\n> --undo-merges options to \"git log\", that might be a nice independent\n> change for the revision option parser.\n\nWait a bit. How would you represent a merge in a patch form that can be\nread by \"git apply\" (and \"patch\") in the first place?\n"},{"id":"149074","messageId":"20100826175234.GB25754@kytes","threadId":"24821","inReplyTo":"7vk4ndo05p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] log/ format-patch improvements","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2010-08-26T17:52:38Z","receivedAt":"2010-08-26T17:52:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano writes:\n> Jonathan Nieder writes:\n> \n> > Ramkumar Ramachandra wrote:\n> > ...\n> >> Good idea. I'll write a patch. Do we also want people to be able to\n> >> turn off `--no-merges`?\n> >\n> > I don't see a need for it.\n> >\n> > However, if you can think of good names for --undo-no-merges and\n> > --undo-merges options to \"git log\", that might be a nice independent\n> > change for the revision option parser.\n> \n> Wait a bit. How would you represent a merge in a patch form that can be\n> read by \"git apply\" (and \"patch\") in the first place?\n\nOh, I'm not attempting that. I merely wanted people to be able to turn\noff `--no-merges` so they can get the current functionality.\n\nAs far as represeting merge commits go, I'm still thinking about it. I\ntalked to Thomas about it briefly on IRC. The main issues:\n1. What's the point? When there's no conflict resolution, a merge\n commit would be empty.\n2. How do we uniquely specify what to merge? We can't use branch names\n or commit SHA1s, because they can change.\n\n-- Ram\n"}]}