{"thread":{"id":"33411","subject":"[PATCH v3 0/4] send-email: couple of improvements","startedAt":"2013-04-07T07:10:26Z","lastAt":"2013-04-07T18:02:41Z","messageCount":21,"participants":["Felipe Contreras","Junio C Hamano","Matthieu Moy"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"213378","messageId":"1365318630-11882-1-git-send-email-felipe.contreras@gmail.com","threadId":"33411","inReplyTo":null,"subject":"[PATCH v3 0/4] send-email: couple of improvements","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T07:10:26Z","receivedAt":"2013-04-07T07:10:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nIntegrated the latest feedback, and improve format-patch along the way. There\nshould be no disadvantages with applying the latest patch.\n\nFelipe Contreras (4):\n send-email: make annotate configurable\n format-patch: improve head calculation for cover-letter\n format-patch: refactor branch name calculation\n format-patch: add format.cover-letter configuration\n\n Documentation/config.txt | 7 ++++\n Documentation/git-format-patch.txt | 5 ++-\n Documentation/git-send-email.txt | 5 ++-\n builtin/log.c | 75 +++++++++++++++++---------------------\n git-send-email.perl | 7 ++--\n 5 files changed, 51 insertions(+), 48 deletions(-)\n\n-- \n1.8.2\n"},{"id":"213379","messageId":"1365318630-11882-2-git-send-email-felipe.contreras@gmail.com","threadId":"33411","inReplyTo":"1365318630-11882-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v3 1/4] send-email: make annotate configurable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T07:10:27Z","receivedAt":"2013-04-07T07:10:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Some people always do --annotate, lets not force them to always type\nthat.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/config.txt | 1 +\n Documentation/git-send-email.txt | 5 +++--\n git-send-email.perl | 7 ++++---\n 3 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bbba728..c8e2178 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1994,6 +1994,7 @@ sendemail..*::\n \n sendemail.aliasesfile::\n sendemail.aliasfiletype::\n+sendemail.annotate::\n sendemail.bcc::\n sendemail.cc::\n sendemail.cccmd::\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 44a1f7c..2facc18 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -45,8 +45,9 @@ Composing\n ~~~~~~~~~\n \n --annotate::\n-\tReview and edit each patch you're about to send. See the\n-\tCONFIGURATION section for 'sendemail.multiedit'.\n+\tReview and edit each patch you're about to send. Default is the value\n+\tof 'sendemail.annotate'. See the CONFIGURATION section for\n+\t'sendemail.multiedit'.\n \n --bcc=
::\n \tSpecify a \"Bcc:\" value for each email. Default is the value of\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex be809e5..e187b12 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -54,7 +54,7 @@ git send-email [options] \n --[no-]bcc * Email Bcc:\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n- --annotate * Review each patch that will be sent in an editor.\n+ --[no-]annotate * Review each patch that will be sent in an editor.\n --compose * Open an editor for introduction.\n --compose-encoding * Encoding to assume for introduction.\n --8bit-encoding * Encoding to assume 8bit mails if undeclared\n@@ -212,7 +212,8 @@ my %config_bool_settings = (\n \"signedoffbycc\" => [\\$signed_off_by_cc, undef],\n \"signedoffcc\" => [\\$signed_off_by_cc, undef], # Deprecated\n \"validate\" => [\\$validate, 1],\n- \"multiedit\" => [\\$multiedit, undef]\n+ \"multiedit\" => [\\$multiedit, undef],\n+ \"annotate\" => [\\$annotate, undef]\n );\n \n my %config_settings = (\n@@ -304,7 +305,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n \t\t \"smtp-debug:i\" => \\$debug_net_smtp,\n \t\t \"smtp-domain:s\" => \\$smtp_domain,\n \t\t \"identity=s\" => \\$identity,\n-\t\t \"annotate\" => \\$annotate,\n+\t\t \"annotate!\" => \\$annotate,\n \t\t \"compose\" => \\$compose,\n \t\t \"quiet\" => \\$quiet,\n \t\t \"cc-cmd=s\" => \\$cc_cmd,\n-- \n1.8.2\n"},{"id":"213380","messageId":"1365318630-11882-3-git-send-email-felipe.contreras@gmail.com","threadId":"33411","inReplyTo":"1365318630-11882-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v3 2/4] format-patch: improve head calculation for cover-letter","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T07:10:28Z","receivedAt":"2013-04-07T07:10:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If we do it after the revision traversal we can be sure that this is\nindeed a commit that will be processed (i.e. not a merge) and it's the\ntop most one (thus removing the NEEDSWORK comment).\n\nSigned-off-by: Felipe Contreras \n---\n builtin/log.c | 20 ++------------------\n 1 file changed, 2 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8f0b2e8..6c78d7f 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1319,24 +1319,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.show_root_diff = 1;\n \n \tif (cover_letter) {\n-\t\t/*\n-\t\t * NEEDSWORK:randomly pick one positive commit to show\n-\t\t * diffstat; this is often the tip and the command\n-\t\t * happens to do the right thing in most cases, but a\n-\t\t * complex command like \"--cover-letter a b c ^bottom\"\n-\t\t * picks \"c\" and shows diffstat between bottom..c\n-\t\t * which may not match what the series represents at\n-\t\t * all and totally broken.\n-\t\t */\n-\t\tint i;\n-\t\tfor (i = 0; i < rev.pending.nr; i++) {\n-\t\t\tstruct object *o = rev.pending.objects[i].item;\n-\t\t\tif (!(o->flags & UNINTERESTING))\n-\t\t\t\thead = (struct commit *)o;\n-\t\t}\n-\t\t/* There is nothing to show; it is not an error, though. */\n-\t\tif (!head)\n-\t\t\treturn 0;\n \t\tif (!branch_name)\n \t\t\tbranch_name = find_branch_name(&rev);\n \t}\n@@ -1372,6 +1354,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n \t\tlist[nr - 1] = commit;\n \t}\n+\tif (nr > 0)\n+\t\thead = list[0];\n \ttotal = nr;\n \tif (!keep_subject && auto_number && total > 1)\n \t\tnumbered = 1;\n-- \n1.8.2\n"},{"id":"213381","messageId":"1365318630-11882-4-git-send-email-felipe.contreras@gmail.com","threadId":"33411","inReplyTo":"1365318630-11882-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v3 3/4] format-patch: refactor branch name calculation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T07:10:29Z","receivedAt":"2013-04-07T07:10:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"By moving the part that relies on rev->pending earlier, where we are\nalready checking the special case where there's only one ref.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/log.c | 33 ++++++++++++++++-----------------\n 1 file changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 6c78d7f..e4fb3c8 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1049,15 +1049,6 @@ static char *find_branch_name(struct rev_info *rev)\n \tif (0 <= positive) {\n \t\tref = rev->cmdline.rev[positive].name;\n \t\ttip_sha1 = rev->cmdline.rev[positive].item->sha1;\n-\t} else if (!rev->cmdline.nr && rev->pending.nr == 1 &&\n-\t\t !strcmp(rev->pending.objects[0].name, \"HEAD\")) {\n-\t\t/*\n-\t\t * No actual ref from command line, but \"HEAD\" from\n-\t\t * rev->def was added in setup_revisions()\n-\t\t * e.g. format-patch --cover-letter -12\n-\t\t */\n-\t\tref = \"HEAD\";\n-\t\ttip_sha1 = rev->pending.objects[0].item->sha1;\n \t} else {\n \t\treturn NULL;\n \t}\n@@ -1288,28 +1279,36 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (rev.pending.nr == 1) {\n+\t\tint check_head = 0;\n+\n \t\tif (rev.max_count < 0 && !rev.show_root_diff) {\n \t\t\t/*\n \t\t\t * This is traditional behaviour of \"git format-patch\n \t\t\t * origin\" that prepares what the origin side still\n \t\t\t * does not have.\n \t\t\t */\n-\t\t\tunsigned char sha1[20];\n-\t\t\tconst char *ref;\n-\n \t\t\trev.pending.objects[0].item->flags |= UNINTERESTING;\n \t\t\tadd_head_to_pending(&rev);\n-\t\t\tref = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n-\t\t\tif (ref && !prefixcmp(ref, \"refs/heads/\"))\n-\t\t\t\tbranch_name = xstrdup(ref + strlen(\"refs/heads/\"));\n-\t\t\telse\n-\t\t\t\tbranch_name = xstrdup(\"\"); /* no branch */\n+\t\t\tcheck_head = 1;\n \t\t}\n \t\t/*\n \t\t * Otherwise, it is \"format-patch -22 HEAD\", and/or\n \t\t * \"format-patch --root HEAD\". The user wants\n \t\t * get_revision() to do the usual traversal.\n \t\t */\n+\n+\t\tif (!strcmp(rev.pending.objects[0].name, \"HEAD\"))\n+\t\t\tcheck_head = 1;\n+\n+\t\tif (check_head) {\n+\t\t\tunsigned char sha1[20];\n+\t\t\tconst char *ref;\n+\t\t\tref = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n+\t\t\tif (ref && !prefixcmp(ref, \"refs/heads/\"))\n+\t\t\t\tbranch_name = xstrdup(ref + strlen(\"refs/heads/\"));\n+\t\t\telse\n+\t\t\t\tbranch_name = xstrdup(\"\"); /* no branch */\n+\t\t}\n \t}\n \n \t/*\n-- \n1.8.2\n"},{"id":"213382","messageId":"1365318630-11882-5-git-send-email-felipe.contreras@gmail.com","threadId":"33411","inReplyTo":"1365318630-11882-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T07:10:30Z","receivedAt":"2013-04-07T07:10:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Also, add a new option: 'auto', so if there's more than one patch, the\ncover letter is generated, otherwise it's not.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-format-patch.txt | 5 +++--\n builtin/log.c | 22 ++++++++++++++++------\n 3 files changed, 25 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c8e2178..c10195c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1092,6 +1092,12 @@ format.signoff::\n the rights to submit this work under the same open source license.\n Please see the 'SubmittingPatches' document for further discussion.\n \n+format.cover-letter::\n+\tAllows to configure the --cover-letter option of format-patch by\n+\tdefault. In addition, you can set it to 'auto' to automatically\n+\tdetermine based on the number of patches (generate if there's more than\n+\tone).\n+\n filter..clean::\n \tThe command which is used to convert the content of a worktree\n \tfile to a blob upon checkin. See linkgit:gitattributes[5] for\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 3a62f50..e1f5730 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,7 +20,7 @@ SYNOPSIS\n \t\t [--ignore-if-in-upstream]\n \t\t [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n-\t\t [--cover-letter] [--quiet] [--notes[=]]\n+\t\t [--[no-]cover-letter] [--quiet] [--notes[=]]\n \t\t []\n \t\t [ | ]\n \n@@ -195,7 +195,7 @@ will want to ensure that threading is disabled for `git send-email`.\n \t`Cc:`, and custom) headers added so far from config or command\n \tline.\n \n---cover-letter::\n+--[no-]cover-letter::\n \tIn addition to the patches, generate a cover letter file\n \tcontaining the shortlog and the overall diffstat. You can\n \tfill in a description in the file before sending it out.\n@@ -260,6 +260,7 @@ attachments, and sign off patches with configuration variables.\n \tcc = \n \tattach [ = mime-boundary-string ]\n \tsignoff = true\n+\tcover-letter = auto\n ------------\n \n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex e4fb3c8..fab7998 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -618,6 +618,7 @@ static void add_header(const char *value)\n #define THREAD_DEEP 2\n static int thread;\n static int do_signoff;\n+static int cover_letter;\n static const char *signature = git_version_string;\n \n static int git_format_config(const char *var, const char *value, void *cb)\n@@ -680,6 +681,17 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(var, \"format.signature\"))\n \t\treturn git_config_string(&signature, var, value);\n+\tif (!strcmp(var, \"format.cover-letter\")) {\n+\t\tif (cover_letter != -1)\n+\t\t\t/* user overrode it */\n+\t\t\treturn 0;\n+\t\tif (value && !strcasecmp(value, \"auto\")) {\n+\t\t\tcover_letter = -1;\n+\t\t\treturn 0;\n+\t\t}\n+\t\tcover_letter = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -1071,7 +1083,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint start_number = -1;\n \tint just_numbers = 0;\n \tint ignore_if_in_upstream = 0;\n-\tint cover_letter = 0;\n \tint boundary_count = 0;\n \tint no_binary_diff = 0;\n \tstruct commit *origin = NULL, *head = NULL;\n@@ -1317,11 +1328,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t */\n \trev.show_root_diff = 1;\n \n-\tif (cover_letter) {\n-\t\tif (!branch_name)\n-\t\t\tbranch_name = find_branch_name(&rev);\n-\t}\n-\n \tif (ignore_if_in_upstream) {\n \t\t/* Don't say anything if head and upstream are the same. */\n \t\tif (rev.pending.nr == 2) {\n@@ -1360,6 +1366,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tnumbered = 1;\n \tif (numbered)\n \t\trev.total = total + start_number - 1;\n+\tif (cover_letter == -1)\n+\t\tcover_letter = (total > 1);\n \tif (in_reply_to || thread || cover_letter)\n \t\trev.ref_message_ids = xcalloc(1, sizeof(struct string_list));\n \tif (in_reply_to) {\n@@ -1371,6 +1379,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_letter) {\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n+\t\tif (!branch_name)\n+\t\t\tbranch_name = find_branch_name(&rev);\n \t\tmake_cover_letter(&rev, use_stdout,\n \t\t\t\t origin, nr, list, head, branch_name, quiet);\n \t\ttotal++;\n-- \n1.8.2\n"},{"id":"213385","messageId":"7vfvz2948b.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"1365318630-11882-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 1/4] send-email: make annotate configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T07:42:12Z","receivedAt":"2013-04-07T07:42:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Some people always do --annotate, lets not force them to always type\n> that.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n\nSounds sensible; any tests to protect this from later breakages?\n\n> Documentation/config.txt | 1 +\n> Documentation/git-send-email.txt | 5 +++--\n> git-send-email.perl | 7 ++++---\n> 3 files changed, 8 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index bbba728..c8e2178 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1994,6 +1994,7 @@ sendemail..*::\n> \n> sendemail.aliasesfile::\n> sendemail.aliasfiletype::\n> +sendemail.annotate::\n> sendemail.bcc::\n> sendemail.cc::\n> sendemail.cccmd::\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index 44a1f7c..2facc18 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -45,8 +45,9 @@ Composing\n> ~~~~~~~~~\n> \n> --annotate::\n> -\tReview and edit each patch you're about to send. See the\n> -\tCONFIGURATION section for 'sendemail.multiedit'.\n> +\tReview and edit each patch you're about to send. Default is the value\n> +\tof 'sendemail.annotate'. See the CONFIGURATION section for\n> +\t'sendemail.multiedit'.\n> \n> --bcc=
::\n> \tSpecify a \"Bcc:\" value for each email. Default is the value of\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index be809e5..e187b12 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -54,7 +54,7 @@ git send-email [options] \n> --[no-]bcc * Email Bcc:\n> --subject * Email \"Subject:\"\n> --in-reply-to * Email \"In-Reply-To:\"\n> - --annotate * Review each patch that will be sent in an editor.\n> + --[no-]annotate * Review each patch that will be sent in an editor.\n> --compose * Open an editor for introduction.\n> --compose-encoding * Encoding to assume for introduction.\n> --8bit-encoding * Encoding to assume 8bit mails if undeclared\n> @@ -212,7 +212,8 @@ my %config_bool_settings = (\n> \"signedoffbycc\" => [\\$signed_off_by_cc, undef],\n> \"signedoffcc\" => [\\$signed_off_by_cc, undef], # Deprecated\n> \"validate\" => [\\$validate, 1],\n> - \"multiedit\" => [\\$multiedit, undef]\n> + \"multiedit\" => [\\$multiedit, undef],\n> + \"annotate\" => [\\$annotate, undef]\n> );\n> \n> my %config_settings = (\n> @@ -304,7 +305,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n> \t\t \"smtp-debug:i\" => \\$debug_net_smtp,\n> \t\t \"smtp-domain:s\" => \\$smtp_domain,\n> \t\t \"identity=s\" => \\$identity,\n> -\t\t \"annotate\" => \\$annotate,\n> +\t\t \"annotate!\" => \\$annotate,\n> \t\t \"compose\" => \\$compose,\n> \t\t \"quiet\" => \\$quiet,\n> \t\t \"cc-cmd=s\" => \\$cc_cmd,\n"},{"id":"213389","messageId":"7vbo9q93u2.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"1365318630-11882-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 2/4] format-patch: improve head calculation for cover-letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T07:50:45Z","receivedAt":"2013-04-07T07:50:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> If we do it after the revision traversal we can be sure that this is\n> indeed a commit that will be processed (i.e. not a merge) and it's the\n> top most one (thus removing the NEEDSWORK comment).\n\nThere may not be a single top-most one anyway (which is what that\n\"randomly pick\" comment refers to and punts), so taking the tip\nafter traversal is just as good as doing it before. So this is a\ngood change, but it still is punting.\n\nThe \"head\" is used only to produce the fake \"From xxxxx \"\nline to mark the output as format-patch output to /etc/magic, so it\ndoes not make sense to do anything fancier, like erroring out with\n\"You specified two or more tips and we cannot pick one\".\n\nSo it is a good thing to punt.\n\nI like this simplification. Good job.\n\n> Signed-off-by: Felipe Contreras \n> ---\n> builtin/log.c | 20 ++------------------\n> 1 file changed, 2 insertions(+), 18 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 8f0b2e8..6c78d7f 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1319,24 +1319,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \trev.show_root_diff = 1;\n> \n> \tif (cover_letter) {\n> \t\tif (!branch_name)\n> \t\t\tbranch_name = find_branch_name(&rev);\n> \t}\n> @@ -1372,6 +1354,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n> \t\tlist[nr - 1] = commit;\n> \t}\n> +\tif (nr > 0)\n> +\t\thead = list[0];\n> \ttotal = nr;\n> \tif (!keep_subject && auto_number && total > 1)\n> \t\tnumbered = 1;\n"},{"id":"213390","messageId":"7v4nfi92q1.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"1365318630-11882-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T08:14:46Z","receivedAt":"2013-04-07T08:14:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Also, add a new option: 'auto', so if there's more than one patch, the\n> cover letter is generated, otherwise it's not.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/config.txt | 6 ++++++\n> Documentation/git-format-patch.txt | 5 +++--\n> builtin/log.c | 22 ++++++++++++++++------\n> 3 files changed, 25 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index c8e2178..c10195c 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1092,6 +1092,12 @@ format.signoff::\n> the rights to submit this work under the same open source license.\n> Please see the 'SubmittingPatches' document for further discussion.\n> \n> +format.cover-letter::\n\nI thought I saw somebody already pointed out that this was wrong.\n\n> +\tAllows to configure the --cover-letter option of format-patch by\n> +\tdefault.\n> +\tIn addition, you can set it to 'auto' to automatically\n> +\tdetermine based on the number of patches (generate if there's more than\n> +\tone).\n\nI find this somewhat a strange phrasing. It _sets_ the default for\nthe cover-letter option, either to true or false or auto, so there\nis no \"in addition\" about it.\n\nPerhaps\n\nformat.coverLetter::\n\tSetting this variable to `true` (or `false`) tells the\n\t`format-patch` command to pretend the `--cover-letter` (or\n\t`--no-cover-letter`) was given from the command line. When\n\tset to `auto`, `format-patch` assumes `--cover-letter` if\n\tand only if it is working on more than one commit.\n\nor something?\n\nIt also is somewhat strange why the command line argument does not\naccept `--cover-letter=auto` option. In general, a configuration\nvariable should not be more flexible mechanism than what the command\nline could give the user. As I already said, I think that would be\na better longer-term default, and that makes it even more stand-out.\n\n> @@ -1360,6 +1366,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \t\tnumbered = 1;\n> \tif (numbered)\n> \t\trev.total = total + start_number - 1;\n> +\tif (cover_letter == -1)\n> +\t\tcover_letter = (total > 1);\n> \tif (in_reply_to || thread || cover_letter)\n> \t\trev.ref_message_ids = xcalloc(1, sizeof(struct string_list));\n> \tif (in_reply_to) {\n> @@ -1371,6 +1379,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tif (cover_letter) {\n> \t\tif (thread)\n> \t\t\tgen_message_id(&rev, \"cover\");\n> +\t\tif (!branch_name)\n> +\t\t\tbranch_name = find_branch_name(&rev);\n> \t\tmake_cover_letter(&rev, use_stdout,\n> \t\t\t\t origin, nr, list, head, branch_name, quiet);\n> \t\ttotal++;\n\nThis part is nicely done, thanks to the previous two steps. You\nmight even want to remove the call to the find_branch_name() helper\nfunction from here, and instead call it from make_cover_letter()\nwhen branch_name is not yet there.\n\nWe would need tests to make sure future changes will not break this.\n"},{"id":"213392","messageId":"CAMP44s1tHdqKmMJbc6_zwEFZ-bK_boSX5zBu1zeBizJ8QVh2uw@mail.gmail.com","threadId":"33411","inReplyTo":"7vbo9q93u2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 2/4] format-patch: improve head calculation for cover-letter","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T08:36:23Z","receivedAt":"2013-04-07T08:36:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 1:50 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> If we do it after the revision traversal we can be sure that this is\n>> indeed a commit that will be processed (i.e. not a merge) and it's the\n>> top most one (thus removing the NEEDSWORK comment).\n>\n> There may not be a single top-most one anyway (which is what that\n> \"randomly pick\" comment refers to and punts), so taking the tip\n> after traversal is just as good as doing it before. So this is a\n> good change, but it still is punting.\n\nYeah, I thought of looking at 'git diff' and do the same thing, but\nthen I realized 'git diff --stat' has the same issue, so I don't think\nit really matters.\n\n-- \nFelipe Contreras\n"},{"id":"213393","messageId":"CAMP44s0rsWCZE=MyW3zuBoVM5ZtEGQQb+deQJB5-i6tBzFdTfw@mail.gmail.com","threadId":"33411","inReplyTo":"7v4nfi92q1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T08:48:14Z","receivedAt":"2013-04-07T08:48:14Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 3:14 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Also, add a new option: 'auto', so if there's more than one patch, the\n>> cover letter is generated, otherwise it's not.\n>>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> Documentation/config.txt | 6 ++++++\n>> Documentation/git-format-patch.txt | 5 +++--\n>> builtin/log.c | 22 ++++++++++++++++------\n>> 3 files changed, 25 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index c8e2178..c10195c 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1092,6 +1092,12 @@ format.signoff::\n>> the rights to submit this work under the same open source license.\n>> Please see the 'SubmittingPatches' document for further discussion.\n>>\n>> +format.cover-letter::\n>\n> I thought I saw somebody already pointed out that this was wrong.\n\nYeah, I forgot.\n\n>> + Allows to configure the --cover-letter option of format-patch by\n>> + default.\n>> + In addition, you can set it to 'auto' to automatically\n>> + determine based on the number of patches (generate if there's more than\n>> + one).\n>\n> I find this somewhat a strange phrasing. It _sets_ the default for\n> the cover-letter option, either to true or false or auto, so there\n> is no \"in addition\" about it.\n\nThere's no --cover-letter=auto.\n\n> Perhaps\n>\n> format.coverLetter::\n> Setting this variable to `true` (or `false`) tells the\n> `format-patch` command to pretend the `--cover-letter` (or\n> `--no-cover-letter`) was given from the command line. When\n> set to `auto`, `format-patch` assumes `--cover-letter` if\n> and only if it is working on more than one commit.\n\nI prefer the wording suggested by Ramkumar with a minor modification:\n\nA boolean that controls whether to generate a cover-letter when\nformat-patch is invoked, but in addition can be set to \"auto\", to\ngenerate a cover-letter only when generating more than one patch.\n\n> or something?\n>\n> It also is somewhat strange why the command line argument does not\n> accept `--cover-letter=auto` option. In general, a configuration\n> variable should not be more flexible mechanism than what the command\n> line could give the user. As I already said, I think that would be\n> a better longer-term default, and that makes it even more stand-out.\n\nUsers know how to count, if the default is no cover-letter, then doing\n--cover-letter=auto is basically --cover-letter, unless they don't\nknow how to count and thought --cover-letter=auto would do something.\nThe same is true is they configured coverletter=true.\n\nI don't see any value in doing that.\n\n> This part is nicely done, thanks to the previous two steps. You\n> might even want to remove the call to the find_branch_name() helper\n> function from here, and instead call it from make_cover_letter()\n> when branch_name is not yet there.\n\nTrue. We can also move the 'head' variable inside make_cover_letter() as well.\n\n> We would need tests to make sure future changes will not break this.\n\nSigh. All right.\n\n-- \nFelipe Contreras\n"},{"id":"213397","messageId":"7vmwta7lx3.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"CAMP44s0rsWCZE=MyW3zuBoVM5ZtEGQQb+deQJB5-i6tBzFdTfw@mail.gmail.com","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T09:03:04Z","receivedAt":"2013-04-07T09:03:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Users know how to count, if the default is no cover-letter, then doing\n> --cover-letter=auto is basically --cover-letter, unless they don't\n> know how to count and thought --cover-letter=auto would do something.\n> The same is true is they configured coverletter=true.\n\nThe user who says \"format-patch origin\" does not necessarily have to\ncount beforehand with \"rev-list origin..\", so \"Users know how to\ncount\" does not sound like a relevant and convincing argument to me.\n"},{"id":"213399","messageId":"CAMP44s1pMLzkQrE1iKmYyj_FOZqkUp46HRQ6kjWfNvOYA5FkVQ@mail.gmail.com","threadId":"33411","inReplyTo":"7vmwta7lx3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T09:11:00Z","receivedAt":"2013-04-07T09:11:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 4:03 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Users know how to count, if the default is no cover-letter, then doing\n>> --cover-letter=auto is basically --cover-letter, unless they don't\n>> know how to count and thought --cover-letter=auto would do something.\n>> The same is true is they configured coverletter=true.\n>\n> The user who says \"format-patch origin\" does not necessarily have to\n> count beforehand with \"rev-list origin..\", so \"Users know how to\n> count\" does not sound like a relevant and convincing argument to me.\n\nSo, I'm a user that does 'git format-patch --cover-letter=auto origin'\nso I don't have the format.coverletter=auto configuration. Why? Why am\nI not setting that configuration, and why am I running\n--cover-letter=auto?\n\nThe only reason I can think of is that I ran 'git format-patch origin'\nand it didn't do what I wanted (or I knew befeorehand).\n\nWhat other reason could the user possibly have?\n\n-- \nFelipe Contreras\n"},{"id":"213406","messageId":"vpqwqsefl9b.fsf@grenoble-inp.fr","threadId":"33411","inReplyTo":"CAMP44s1pMLzkQrE1iKmYyj_FOZqkUp46HRQ6kjWfNvOYA5FkVQ@mail.gmail.com","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-07T14:50:08Z","receivedAt":"2013-04-07T14:50:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras writes:\n\n> So, I'm a user that does 'git format-patch --cover-letter=auto origin'\n> so I don't have the format.coverletter=auto configuration. Why? Why am\n> I not setting that configuration, and why am I running\n> --cover-letter=auto?\n\nThe command may well be ran from a script or alias. It is good in\ngeneral to give the user/script writer a simple way to get predictible\nbehavior regardless of the configuration.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"213417","messageId":"CAMP44s2TvC9Ek51GyhPqtv7b-w8u8kr=tQsmdWEVGOycb6MEqA@mail.gmail.com","threadId":"33411","inReplyTo":"vpqwqsefl9b.fsf@grenoble-inp.fr","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T16:03:15Z","receivedAt":"2013-04-07T16:03:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 9:50 AM, Matthieu Moy\n wrote:\n> Felipe Contreras writes:\n>\n>> So, I'm a user that does 'git format-patch --cover-letter=auto origin'\n>> so I don't have the format.coverletter=auto configuration. Why? Why am\n>> I not setting that configuration, and why am I running\n>> --cover-letter=auto?\n>\n> The command may well be ran from a script or alias.\n\nWouldn't this work for both cases?\n\n% git -c format.coverletter=auto format-patch\n\n> It is good in\n> general to give the user/script writer a simple way to get predictible\n> behavior regardless of the configuration.\n\nPredictable means when the user does X, (s)he would know what would be\nthe outcome. But if the user never does X, it doesn't matter.\n\n-- \nFelipe Contreras\n"},{"id":"213419","messageId":"vpqbo9qfh1y.fsf@grenoble-inp.fr","threadId":"33411","inReplyTo":"CAMP44s2TvC9Ek51GyhPqtv7b-w8u8kr=tQsmdWEVGOycb6MEqA@mail.gmail.com","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-04-07T16:20:57Z","receivedAt":"2013-04-07T16:20:57Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras writes:\n\n> On Sun, Apr 7, 2013 at 9:50 AM, Matthieu Moy\n> wrote:\n>> Felipe Contreras writes:\n>>\n>>> So, I'm a user that does 'git format-patch --cover-letter=auto origin'\n>>> so I don't have the format.coverletter=auto configuration. Why? Why am\n>>> I not setting that configuration, and why am I running\n>>> --cover-letter=auto?\n>>\n>> The command may well be ran from a script or alias.\n>\n> Wouldn't this work for both cases?\n>\n> % git -c format.coverletter=auto format-patch\n\nThen, what's the point in having a --cover-letter option?\n\n\"git -c\" is a good last-chance solution, but when we provide the same\nfeature as a command-line option and as a configuration option, I can\nsee no reason to add subtle difference between them.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"213421","messageId":"CAMP44s1bU4RE+h49JDgoG47O8ySv2b115wubgN1t68A_ObNW4g@mail.gmail.com","threadId":"33411","inReplyTo":"vpqbo9qfh1y.fsf@grenoble-inp.fr","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T16:43:07Z","receivedAt":"2013-04-07T16:43:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 11:20 AM, Matthieu Moy\n wrote:\n> Felipe Contreras writes:\n>\n>> On Sun, Apr 7, 2013 at 9:50 AM, Matthieu Moy\n>> wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> So, I'm a user that does 'git format-patch --cover-letter=auto origin'\n>>>> so I don't have the format.coverletter=auto configuration. Why? Why am\n>>>> I not setting that configuration, and why am I running\n>>>> --cover-letter=auto?\n>>>\n>>> The command may well be ran from a script or alias.\n>>\n>> Wouldn't this work for both cases?\n>>\n>> % git -c format.coverletter=auto format-patch\n>\n> Then, what's the point in having a --cover-letter option?\n\nIt's useful in the opposite cases: when the command *not* run from a\nscript, or an alias.\n\n-- \nFelipe Contreras\n"},{"id":"213422","messageId":"CAMP44s0g5YfvprwukJ4rnqXrm+S9-KJkwyYB+sxAcQjZpDXufg@mail.gmail.com","threadId":"33411","inReplyTo":"7vfvz2948b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/4] send-email: make annotate configurable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T16:45:28Z","receivedAt":"2013-04-07T16:45:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 1:42 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Some people always do --annotate, lets not force them to always type\n>> that.\n>>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>\n> Sounds sensible; any tests to protect this from later breakages?\n\nGiven that annotate doesn't even have tests, I don't think it's needed\nat this point.\n\n-- \nFelipe Contreras\n"},{"id":"213425","messageId":"7v38v25k8b.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"vpqbo9qfh1y.fsf@grenoble-inp.fr","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T17:22:28Z","receivedAt":"2013-04-07T17:22:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>> Wouldn't this work for both cases?\n>>\n>> % git -c format.coverletter=auto format-patch\n>\n> Then, what's the point in having a --cover-letter option?\n>\n> \"git -c\" is a good last-chance solution, but when we provide the same\n> feature as a command-line option and as a configuration option, I can\n> see no reason to add subtle difference between them.\n\nI do not see a good reason to resist consistency by suggesting a\nworkaround. We may do so ourselves when responding to an end user\nbug report \"I cannot countermand X from the command line\" as a first\nresponse \"In the meantime you can work it around like so\", but that\nis as you said a last-resort workaround and does not justify why the\ncommand line interface is lacking an obvious override.\n\nBut I think --cover-letter=auto I made was a bad suggestion. An\noptional parameter to a command line option is generally bad, as it\nmakes the parsing ambiguous [*1*], and turning what traditionally\nwas a boolean without any parameter into one that takes an optional\nparameter is even worse.\n\nWe should instead add this as a new feature, --auto-cover-letter, or\nsomething, i.e. the synopsis will have these as the choices\n\n [--[no-]cover-letter|--auto-cover-letter]\n\nand the last one would win (e.g. \"--cover-letter --auto-cover-letter\" \nwould mean \"auto\").\n\n\n[Footnote]\n\n*1* Does \"--cover-letter yes\" mean \"--cover-letter=yes\" aka \"please\nuse cover letter\" or is it \"--cover-letter\" (please use cover\nletter) followed by an unrelated command line argument \"yes\"?\n"},{"id":"213426","messageId":"7vvc7y45nf.fsf@alter.siamese.dyndns.org","threadId":"33411","inReplyTo":"CAMP44s0g5YfvprwukJ4rnqXrm+S9-KJkwyYB+sxAcQjZpDXufg@mail.gmail.com","subject":"Re: [PATCH v3 1/4] send-email: make annotate configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-07T17:22:44Z","receivedAt":"2013-04-07T17:22:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, Apr 7, 2013 at 1:42 AM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> Some people always do --annotate, lets not force them to always type\n>>> that.\n>>>\n>>> Signed-off-by: Felipe Contreras \n>>> ---\n>>\n>> Sounds sensible; any tests to protect this from later breakages?\n>\n> Given that annotate doesn't even have tests, I don't think it's needed\n> at this point.\n\nThat is a good way to spread earlier mistakes and add more instance\nof the same kind of mistakes into the future.\n\nIt is perfectly fine to say \"I ran out my Git time for the change\nitself; help by others to add test for it is appreciated.\" It is\nnot reasonable to say \"See there is the same mistake already in the\ncode, so I am free to make the same mistake, you cannot hold it\nagainst me, and this feature should never be protected by tests\".\n"},{"id":"213443","messageId":"CAMP44s3bdFfZRGL1BHtODiad6ZXD_xy1MfUhNPrJQV=L4hGamw@mail.gmail.com","threadId":"33411","inReplyTo":"7vvc7y45nf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/4] send-email: make annotate configurable","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T17:58:22Z","receivedAt":"2013-04-07T17:58:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 12:22 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Sun, Apr 7, 2013 at 1:42 AM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>\n>>>> Some people always do --annotate, lets not force them to always type\n>>>> that.\n>>>>\n>>>> Signed-off-by: Felipe Contreras \n>>>> ---\n>>>\n>>> Sounds sensible; any tests to protect this from later breakages?\n>>\n>> Given that annotate doesn't even have tests, I don't think it's needed\n>> at this point.\n>\n> That is a good way to spread earlier mistakes and add more instance\n> of the same kind of mistakes into the future.\n>\n> It is perfectly fine to say \"I ran out my Git time for the change\n> itself; help by others to add test for it is appreciated.\" It is\n> not reasonable to say \"See there is the same mistake already in the\n> code, so I am free to make the same mistake, you cannot hold it\n> against me, and this feature should never be protected by tests\".\n\nI did not say that, I said I think this is not needed at this point,\nif you think otherwise, you can go ahead and implement it. I think the\npatch is perfectly fine as it is, and I'm not going to to add tests\nfor something that is potentially time consuming for me, and this\npatch is not introducing, that is; the annotate feature. If we\nsurvived without tests for annotate, we can survive without tests for\nsendemail.annotate.\n\n-- \nFelipe Contreras\n"},{"id":"213444","messageId":"CAMP44s1Pd_O8FmWy=bgzEDrbGQuNStn9=LSYF146VCGLby3F7g@mail.gmail.com","threadId":"33411","inReplyTo":"7v38v25k8b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 4/4] format-patch: add format.cover-letter configuration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-07T18:02:41Z","receivedAt":"2013-04-07T18:02:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 7, 2013 at 12:22 PM, Junio C Hamano wrote:\n> Matthieu Moy writes:\n>\n>>> Wouldn't this work for both cases?\n>>>\n>>> % git -c format.coverletter=auto format-patch\n>>\n>> Then, what's the point in having a --cover-letter option?\n>>\n>> \"git -c\" is a good last-chance solution, but when we provide the same\n>> feature as a command-line option and as a configuration option, I can\n>> see no reason to add subtle difference between them.\n>\n> I do not see a good reason to resist consistency by suggesting a\n> workaround.\n\nThere's no need for a workaround, because there's no use-case.\n\n> We may do so ourselves when responding to an end user\n> bug report \"I cannot countermand X from the command line\" as a first\n> response \"In the meantime you can work it around like so\", but that\n> is as you said a last-resort workaround and does not justify why the\n> command line interface is lacking an obvious override.\n\nThe command line interface is not missing anything.\n\n> But I think --cover-letter=auto I made was a bad suggestion. An\n> optional parameter to a command line option is generally bad, as it\n> makes the parsing ambiguous [*1*], and turning what traditionally\n> was a boolean without any parameter into one that takes an optional\n> parameter is even worse.\n>\n> We should instead add this as a new feature, --auto-cover-letter, or\n> something, i.e. the synopsis will have these as the choices\n>\n> [--[no-]cover-letter|--auto-cover-letter]\n>\n> and the last one would win (e.g. \"--cover-letter --auto-cover-letter\"\n> would mean \"auto\").\n\nWell, go ahead and implement it, because I'm not going to waste my\ntime implementing something nobody will *ever* use.\n\n-- \nFelipe Contreras\n"}]}