{"thread":{"id":"18938","subject":"[PATCH 1/6] format-patch: add cover{letter,onepatch} options","startedAt":"2009-04-18T16:16:15Z","lastAt":"2009-05-05T15:23:38Z","messageCount":21,"participants":["Frank Terbeck","Junio C Hamano","Jeff King","Stephen Boyd"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"111585","messageId":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":null,"subject":"[PATCH 0/6] more automation for cover letter generation","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:15Z","receivedAt":"2009-04-18T16:16:15Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"I like cover letters, in fact I like them enough to always want\n--cover-letter to format-patch. The problem with that are patch \"series\"\nthat are only one patch long, where a cover letter would feel silly.\n\nFor now, I solved that by using a shell function that wrapped around\nformat-patch and did the trick for me.\n\nWith this series, format-patch can do it and do it better than my\nwrapper could.\n\nThe following setup would suit me pretty well:\n\n[format]\n coverletter = true\n coveronepatch = false\n overwritecoverletter = false\n\n\nThe series is based on master and doesn't seem to break anything\nwithin the test suite. It could maybe use own tests, but I must admit\nthat I didn't look too closely at how git's test suite works and where\nto put in tests for this.\n\nFinally, this series does not change anything about format-patch's\ndefault behaviour.\n\nRegards, Frank\n\n\nFrank Terbeck (6):\n format-patch: add cover{letter,onepatch} options\n\n The above makes it possible to *always* create cover letters\n without requesting it via --cover-letter. You can also suppress\n cover letters for patch series, that are just one patch long,\n where you'd probably put anything you have to say in addition\n between '---' and the diffstat.\n\n Add documentation for format-patch's --cover-one-patch\n Document format.coverletter and format.coveronepatch\n\n format-patch: introduce overwritecoverletter option\n\n When you're always generating cover letters, it can be a good idea\n to not overwrite an existing cover letter, especially if you're\n just refreshing your patch series, or added more commits to it -\n were comments you may have made in an existing cover letter might\n be valuable for the new patch series as well. Therefore setting\n overwritecoverletter to false protects you from overwriting an\n existing cover letter.\n\n I had hoped that this change would be possible without changes in so\n many places, but I didn't see a straight forward one.\n\n Add documentation for --cover-overwrite\n Document format.overwritecoverletter\n\n Documentation/config.txt | 15 +++++++++++++\n Documentation/git-format-patch.txt | 11 +++++++++\n builtin-log.c | 40 +++++++++++++++++++++++++++++++----\n log-tree.c | 9 +++++--\n log-tree.h | 4 +-\n 5 files changed, 69 insertions(+), 10 deletions(-)\n"},{"id":"111583","messageId":"1240071381-25165-2-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 1/6] format-patch: add cover{letter,onepatch} options","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:16Z","receivedAt":"2009-04-18T16:16:16Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"When using --cover-letter, a cover letter is created even if the patch\nseries is only one patch long. By setting format.coveronepatch to 'false', this\nis prevented. To temporarily force creating cover letter even for\none-patch \"series\", the --cover-one-patch option may be used.\n\nTo always create cover letters, the format.coverletter option may be set\nto 'true'.\n\nA possible setup to create cover letters for every created patch series,\nthat is longer than one patch, would be:\n\n[format]\n coverletter = true\n coveronepatch = false\n\ngit-format-patch's default behaviour is not altered by this patch.\n\nSigned-off-by: Frank Terbeck \n---\n builtin-log.c | 16 +++++++++++++++-\n 1 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..82d8724 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -436,6 +436,9 @@ static char **extra_cc;\n static int extra_cc_nr;\n static int extra_cc_alloc;\n \n+static int cover_letter = 0;\n+static int cover_one_patch = 1;\n+\n static void add_header(const char *value)\n {\n \tint len = strlen(value);\n@@ -512,6 +515,14 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\tdo_signoff = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.coverletter\")) {\n+\t\tcover_letter = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"format.coveronepatch\")) {\n+\t\tcover_one_patch = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -752,7 +763,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint numbered_files = 0;\t\t/* _just_ numbers */\n \tint subject_prefix = 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@@ -868,6 +878,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tfmt_patch_suffix = argv[i] + 9;\n \t\telse if (!strcmp(argv[i], \"--cover-letter\"))\n \t\t\tcover_letter = 1;\n+\t\telse if (!strcmp(argv[i], \"--cover-one-patch\"))\n+\t\t\tcover_one_patch = 1;\n \t\telse if (!strcmp(argv[i], \"--no-binary\"))\n \t\t\tno_binary_diff = 1;\n \t\telse if (!prefixcmp(argv[i], \"--add-header=\"))\n@@ -1010,6 +1022,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tlist[nr - 1] = commit;\n \t}\n \ttotal = nr;\n+\tif (!cover_one_patch && total == 1)\n+\t\tcover_letter = 0;\n \tif (!keep_subject && auto_number && total > 1)\n \t\tnumbered = 1;\n \tif (numbered)\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111584","messageId":"1240071381-25165-3-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 2/6] Add documentation for format-patch's --cover-one-patch","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:17Z","receivedAt":"2009-04-18T16:16:17Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Signed-off-by: Frank Terbeck \n---\n Documentation/git-format-patch.txt | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex eb2fbcf..65e4089 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,6 +20,7 @@ SYNOPSIS\n \t\t [--subject-prefix=Subject-Prefix]\n \t\t [--cc=]\n \t\t [--cover-letter]\n+\t\t [--cover-one-patch]\n \t\t [ | ]\n \n DESCRIPTION\n@@ -167,6 +168,11 @@ if that is not set.\n \tcontaining the shortlog and the overall diffstat. You can\n \tfill in a description in the file before sending it out.\n \n+--cover-one-patch::\n+\tIf a patch series is only one patch long, the format.coveronepatch\n+\twill prevent the creation of cover letters, if disabled.\n+\tThis option can force creating a cover letter in those cases.\n+\n --suffix=.::\n \tInstead of using `.patch` as the suffix for generated\n \tfilenames, use specified suffix. A common alternative is\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111588","messageId":"1240071381-25165-4-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 3/6] Document format.coverletter and format.coveronepatch","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:18Z","receivedAt":"2009-04-18T16:16:18Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Signed-off-by: Frank Terbeck \n---\n Documentation/config.txt | 10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5319df5..00b8f1f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -705,6 +705,16 @@ fetch.unpackLimit::\n \tespecially on slow filesystems. If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+format.coverletter::\n+\tA boolean variable, if enabled causes the creation of cover letters\n+\tfor patch series without manually using the --cover-letter option\n+\tof linkgit:git-format-patch[1]. This variable defaults to \"false\".\n+\n+format.coveronepatch::\n+\tIf a patch series consists of only one patch, this boolean variable\n+\tdecides whether to create a cover letter or not (if that is\n+\trequested). The default is \"true\".\n+\n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\n \tsubjects. It defaults to \"auto\" which enables it only if there\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111587","messageId":"1240071381-25165-5-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 4/6] format-patch: introduce overwritecoverletter option","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:19Z","receivedAt":"2009-04-18T16:16:19Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"This option, when set, causes a fatal error if format-patch would\noverwrite an existing coverletter to avoid information loss if the\nletter was already edited before.\n\nThis changes the return value of get_patch_filename() from void to int\nand its prototype now contains a new integer parameter 'check'. If\n'check' is false, get_patch_filename() behaves as it did before and it\nwill always return zero. If 'check' is true, get_patch_filename() will\nuse access(3) to check whether the filename it created exists, if so\nget_patch_filename() returns 1; zero otherwise.\n\nThis access(3) test is only done if the overwritecoverletter option is\nunset and reopen_stdout()'s new 'check' parameter is true. That is done\nonly by make_cover_letter().\n\nThe default for overwritecoverletter is true, which means, that the\ndefault behaviour does not change. If overwritecoverletter was unset,\nthere is the new --cover-overwrite option to temporarily force\noverwriting cover letters.\n\nSigned-off-by: Frank Terbeck \n---\n builtin-log.c | 24 ++++++++++++++++++++----\n log-tree.c | 9 ++++++---\n log-tree.h | 4 ++--\n 3 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 82d8724..816b4c9 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -438,6 +438,7 @@ static int extra_cc_alloc;\n \n static int cover_letter = 0;\n static int cover_one_patch = 1;\n+static int cover_overwrite = 1;\n \n static void add_header(const char *value)\n {\n@@ -523,6 +524,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\tcover_one_patch = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.overwritecoverletter\")) {\n+\t\tcover_overwrite = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -531,7 +536,7 @@ static FILE *realstdout = NULL;\n static const char *output_directory = NULL;\n static int outdir_offset;\n \n-static int reopen_stdout(struct commit *commit, struct rev_info *rev)\n+static int reopen_stdout(struct commit *commit, struct rev_info *rev, int check)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n@@ -545,7 +550,16 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev)\n \t\t\tstrbuf_addch(&filename, '/');\n \t}\n \n-\tget_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);\n+\tif (!cover_overwrite && check) {\n+\t\tint exists;\n+\t\texists = get_patch_filename(commit, rev->nr, fmt_patch_suffix,\n+\t\t\t\t\t check, &filename);\n+\t\tif (exists)\n+\t\t\tdie(\"Not overwriting existing coverletter: %s\",\n+\t\t\t filename.buf);\n+\t} else\n+\t\tget_patch_filename(commit, rev->nr, fmt_patch_suffix,\n+\t\t\t\t 0, &filename);\n \n \tif (!DIFF_OPT_TST(&rev->diffopt, QUIET))\n \t\tfprintf(realstdout, \"%s\\n\", filename.buf + outdir_offset);\n@@ -656,7 +670,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\tsha1_to_hex(head->object.sha1), committer, committer);\n \t}\n \n-\tif (!use_stdout && reopen_stdout(commit, rev))\n+\tif (!use_stdout && reopen_stdout(commit, rev, 1))\n \t\treturn;\n \n \tif (commit) {\n@@ -880,6 +894,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tcover_letter = 1;\n \t\telse if (!strcmp(argv[i], \"--cover-one-patch\"))\n \t\t\tcover_one_patch = 1;\n+\t\telse if (!strcmp(argv[i], \"--cover-overwrite\"))\n+\t\t\tcover_overwrite = 1;\n \t\telse if (!strcmp(argv[i], \"--no-binary\"))\n \t\t\tno_binary_diff = 1;\n \t\telse if (!prefixcmp(argv[i], \"--add-header=\"))\n@@ -1086,7 +1102,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,\n-\t\t\t\t\t\t &rev))\n+\t\t\t\t\t\t &rev, 0))\n \t\t\tdie(\"Failed to create output files\");\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree(commit->buffer);\ndiff --git a/log-tree.c b/log-tree.c\nindex 5bd29e6..7163abb 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -179,8 +179,8 @@ static int has_non_ascii(const char *s)\n \treturn 0;\n }\n \n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf)\n+int get_patch_filename(struct commit *commit, int nr, const char *suffix,\n+\t\t\tint check, struct strbuf *buf)\n {\n \tint suffix_len = strlen(suffix) + 1;\n \tint start_len = buf->len;\n@@ -194,6 +194,9 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n \t\t\tstrbuf_setlen(buf, max_len);\n \t\tstrbuf_addstr(buf, suffix);\n \t}\n+\tif (check && (access(buf->buf, F_OK) == 0))\n+\t\treturn 1;\n+\treturn 0;\n }\n \n void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n@@ -262,7 +265,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\textra_headers = subject_buffer;\n \n \t\tget_patch_filename(opt->numbered_files ? NULL : commit, opt->nr,\n-\t\t\t\t opt->patch_suffix, &filename);\n+\t\t\t\t opt->patch_suffix, 0, &filename);\n \t\tsnprintf(buffer, sizeof(buffer) - 1,\n \t\t\t \"\\n--%s%s\\n\"\n \t\t\t \"Content-Type: text/x-patch;\"\ndiff --git a/log-tree.h b/log-tree.h\nindex 20b5caf..8e91b7a 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -20,7 +20,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n void load_ref_decorations(void);\n \n #define FORMAT_PATCH_NAME_MAX 64\n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf);\n+int get_patch_filename(struct commit *commit, int nr, const char *suffix,\n+\t\t int check, struct strbuf *buf);\n \n #endif\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111589","messageId":"1240071381-25165-6-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 5/6] Add documentation for --cover-overwrite","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:20Z","receivedAt":"2009-04-18T16:16:20Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Signed-off-by: Frank Terbeck \n---\n Documentation/git-format-patch.txt | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 65e4089..07a2ee3 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -21,6 +21,7 @@ SYNOPSIS\n \t\t [--cc=]\n \t\t [--cover-letter]\n \t\t [--cover-one-patch]\n+\t\t [--cover-overwrite]\n \t\t [ | ]\n \n DESCRIPTION\n@@ -173,6 +174,10 @@ if that is not set.\n \twill prevent the creation of cover letters, if disabled.\n \tThis option can force creating a cover letter in those cases.\n \n+--cover-overwrite::\n+\tIf format.overwritecoverletter is unset, this option will\n+\ttemporarily force overwriting cover letters.\n+\n --suffix=.::\n \tInstead of using `.patch` as the suffix for generated\n \tfilenames, use specified suffix. A common alternative is\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111586","messageId":"1240071381-25165-7-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"[PATCH 6/6] Document format.overwritecoverletter","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-04-18T16:16:21Z","receivedAt":"2009-04-18T16:16:21Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Signed-off-by: Frank Terbeck \n---\n Documentation/config.txt | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 00b8f1f..616bfb5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -715,6 +715,11 @@ format.coveronepatch::\n \tdecides whether to create a cover letter or not (if that is\n \trequested). The default is \"true\".\n \n+format.overwritecoverletter::\n+\tIf this boolean variable is set to \"false\", it will cause\n+\tlinkgit:git-format-patch[1] fail fatally if it would overwrite an\n+\texisting coverletter. The default is \"true\".\n+\n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\n \tsubjects. It defaults to \"auto\" which enables it only if there\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"111609","messageId":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"Re: [PATCH 0/6] more automation for cover letter generation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-18T18:31:31Z","receivedAt":"2009-04-18T18:31:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Terbeck writes:\n\n> The following setup would suit me pretty well:\n>\n> [format]\n> coverletter = true\n> coveronepatch = false\n\nNobody wants a cover letter to a single patch, so a better way\nwould probably be:\n\n\t'yes' means default behaviour, that is add cover letter for\n\tmultiple-patch series, non for a single patch;\n\n\t'no' means no cover; and\n\n\t'always' means a probably insane \"cover even a single patch\".\n\nIn any case, because this new feature is way too late to be in the\nupcoming 1.6.3 release anyway, I think it is a saner approach to add a\ncommand line option \"--cover=yes\" to \"cover if multiple\", \"--cover=always\"\nto \"cover even a single patch\", and \"--cover=no\" to countermand a\nconfigured \"format.cover\" the user may have in the configuration from the\ncommand line.\n\n> overwritecoverletter = false\n\nI do not think it is particularly a good idea, and it is a good idea to\nhave it in the configuration.\n\n - Why not protect the earlier patch output? People often tweak messages\n (both above and below the three-dash lines) in them.\n\n - Isn't this pretty much per invocation?\n\nI can understand (I may not be enthused about it) a new --clobber={yes,no}\ncommand line option to allow/forbid clobbering the existing files, and you\nmay want to add --clobber=patches to allow clobbering only the patches but\nnot cover (which I do not think makes much sense, though).\n"},{"id":"111834","messageId":"20090421033213.GA14881@coredump.intra.peff.net","threadId":"18938","inReplyTo":"1240071381-25165-1-git-send-email-ft@bewatermyfriend.org","subject":"Re: [PATCH 0/6] more automation for cover letter generation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T03:32:13Z","receivedAt":"2009-04-21T03:32:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 18, 2009 at 06:16:15PM +0200, Frank Terbeck wrote:\n\n> I like cover letters, in fact I like them enough to always want\n> --cover-letter to format-patch. The problem with that are patch \"series\"\n> that are only one patch long, where a cover letter would feel silly.\n> \n> For now, I solved that by using a shell function that wrapped around\n> format-patch and did the trick for me.\n> \n> With this series, format-patch can do it and do it better than my\n> wrapper could.\n\nThat seems like a reasonable goal.\n\n> The following setup would suit me pretty well:\n> \n> [format]\n> coverletter = true\n> coveronepatch = false\n> overwritecoverletter = false\n\nSomething about \"coveronepatch\" seems a bit hack-ish to me. Perhaps it\nshould be \"generate cover letter if there are more than N patches\". You\ncould even just overload \"format.coverletter\" as:\n\n true - always generate cover letter\n false - never generate cover letter\n - generate if there are at least patches\n\n?\n\n> The series is based on master and doesn't seem to break anything\n> within the test suite. It could maybe use own tests, but I must admit\n> that I didn't look too closely at how git's test suite works and where\n> to put in tests for this.\n\nThe tests below were not actually run (and you can see they are based on\nwhat I proposed above, not your existing patches), but they should give\nhopefully give you a headstart.\n\n---\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 11061dd..9e13ee9 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -151,6 +151,27 @@ test_expect_success 'multiple files' '\n \tls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch\n '\n \n+test_expect_success 'format.coverletter=true generates cover letter' '\n+\trm -rf patches &&\n+\tgit config core.coverletter true\n+\tgit format-patch -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverletter=number generates cover letter' '\n+\trm -rf patches &&\n+\tgit config core.coverletter 3 &&\n+\tgit format-patch -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverletter respects minimum patchset size' '\n+\trm -rf patches &&\n+\tgit config core.coverletter 4 &&\n+\tgit format-patch -o patches/ master &&\n+\t! test -f patches/0000-cover-letter.patch\n+'\n+\n check_threading () {\n \texpect=\"$1\" &&\n \tshift &&\n"},{"id":"112966","messageId":"1241431142-8444-1-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2 0/4] more automation for cover letter generation","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T09:58:58Z","receivedAt":"2009-05-04T09:58:58Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Sorry for taking this long, I was covered in work.\n\nJunio C Hamano wrote:\n> Frank Terbeck writes:\n> > [format]\n> > coverletter = true\n> > coveronepatch = false\n> \n> Nobody wants a cover letter to a single patch, so a better way\n> would probably be:\n> \n> \t'yes' means default behaviour, that is add cover letter for\n> \tmultiple-patch series, non for a single patch;\n> \n> \t'no' means no cover; and\n> \n> \t'always' means a probably insane \"cover even a single patch\".\n> \n> In any case, because this new feature is way too late to be in the\n> upcoming 1.6.3 release anyway, I think it is a saner approach to add a\n> command line option \"--cover=yes\" to \"cover if multiple\", \"--cover=always\"\n> to \"cover even a single patch\", and \"--cover=no\" to countermand a\n> configured \"format.cover\" the user may have in the configuration from the\n> command line.\n\nYou're right. And so is Jeff.\nI'll send a followup, that will address comments of the both of you.\nSee below.\n\n> \n> > overwritecoverletter = false\n> \n> I do not think it is particularly a good idea, and it is a good idea to\n> have it in the configuration.\n> \n> - Why not protect the earlier patch output? People often tweak messages\n> (both above and below the three-dash lines) in them.\n> \n> - Isn't this pretty much per invocation?\n> \n> I can understand (I may not be enthused about it) a new --clobber={yes,no}\n> command line option to allow/forbid clobbering the existing files, and you\n> may want to add --clobber=patches to allow clobbering only the patches but\n> not cover (which I do not think makes much sense, though).\n\nThis was a followup idea, which I thought was a reasonable. In the\nmeantime, I feel this is not needed. I'd agree that a more generic\n--clobber option would make sense but frankly, protecting overwrites at\nthis stage is not something worth putting much effort into, IMHO.\n\nJeff King wrote:\n> On Sat, Apr 18, 2009 at 06:16:15PM +0200, Frank Terbeck wrote:\n[...]\n> Something about \"coveronepatch\" seems a bit hack-ish to me. Perhaps it\n> should be \"generate cover letter if there are more than N patches\". You\n> could even just overload \"format.coverletter\" as:\n> \n> true - always generate cover letter\n> false - never generate cover letter\n> - generate if there are at least patches\n\nYeah, a lot better.\nAs Junio said, noone wants cover letters for single patches.\nTherefore, I made format.coverletter default to 2. Setting it to one\nmakes --cover-letter produce for cover letter for single patches, too.\nSetting it to 0 disables cover letters, even with --cover-letter. Any\nother value sets the minimum size of patch series that will trigger\n--cover-letter to create a cover letter.\n\nThat can be altered from the command-line via:\n --cover-letter=always\n --cover-letter=never\n --cover-letter=\n\nIn addition to that I did add format.coverauto.\nSetting it to true (it defaults to false) makes format-patch behave like\nthe user gave the --cover-letter option - with the following exception:\nif --stdout is specified, cover letters are disabled. To force cover\nletters in that case too, --cover-letter needs to be specified *after*\n--stdout.\n\nNow that I think of it again, two weeks after writing this mail\noriginally, I guess it would be possible to drop format.coverauto\naltogether and tell users to use:\n\n % git config --global alias.fp format-patch --cover-letter\n\nI don't know which solution would be preferred. If it's the user-should-\nuse-an-alias approach, I'll create a series that gets rid of\nformat.coverauto changes.\n\n[...]\n> > The series is based on master and doesn't seem to break anything\n> > within the test suite. It could maybe use own tests, but I must admit\n> > that I didn't look too closely at how git's test suite works and where\n> > to put in tests for this.\n> \n> The tests below were not actually run (and you can see they are based on\n> what I proposed above, not your existing patches), but they should give\n> hopefully give you a headstart.\n\nYes, it did. Thank you.\n\nFrank Terbeck (4):\nThe new options and changes:\n Add format.coverletter option\n Add format.coverauto boolean\n\nTests, thanks to Jeff:\n Add tests for coverauto, coverletter and --cover-letter\n\nAnd finally documentation.\n Documentation for format.coverletter and --cover-letter\n\n Documentation/config.txt | 9 +++++\n Documentation/git-format-patch.txt | 17 ++++++++--\n builtin-log.c | 32 +++++++++++++++++--\n t/t3400-rebase.sh | 1 +\n t/t4014-format-patch.sh | 58 ++++++++++++++++++++++++++++++++++-\n 5 files changed, 108 insertions(+), 9 deletions(-)\n"},{"id":"112968","messageId":"1241431142-8444-2-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2 1/4] Add format.coverletter option","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T09:58:59Z","receivedAt":"2009-05-04T09:58:59Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"When using --cover-letter, a cover letter is created even if the patch\nseries is only one patch long.\n\nThis patch cures that by making the behaviour configurable.\n'format.coverletter' is an integer option, that tells git how long a\npatch series needs to be for it to generate a cover letter.\nIf set to 0, git never creates a cover letter.\n\nThe default minimum patch series length is 2.\n\nThe --cover-letter option now accepts integer arguments (such as\n--cover-letter=5), which have the same effect as setting the\n'format.coverletter' option. 'never' instead of an integer has the\nsame effect as 0 has; it surpresses cover letter generation.\n'always' has the same effect as setting the minimum length to 1.\n\nSigned-off-by: Frank Terbeck \n---\n builtin-log.c | 23 +++++++++++++++++++++--\n 1 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..157c8cf 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -436,6 +436,9 @@ static char **extra_cc;\n static int extra_cc_nr;\n static int extra_cc_alloc;\n \n+static int cover_letter = 0;\n+static int cover_letter_len = 2;\n+\n static void add_header(const char *value)\n {\n \tint len = strlen(value);\n@@ -480,6 +483,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\")) {\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.coverletter\")) {\n+\t\tcover_letter_len = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.numbered\")) {\n \t\tif (value && !strcasecmp(value, \"auto\")) {\n \t\t\tauto_number = 1;\n@@ -752,7 +759,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint numbered_files = 0;\t\t/* _just_ numbers */\n \tint subject_prefix = 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@@ -868,7 +874,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tfmt_patch_suffix = argv[i] + 9;\n \t\telse if (!strcmp(argv[i], \"--cover-letter\"))\n \t\t\tcover_letter = 1;\n-\t\telse if (!strcmp(argv[i], \"--no-binary\"))\n+\t\telse if (!prefixcmp(argv[i], \"--cover-letter=always\")) {\n+\t\t\tcover_letter = 1;\n+\t\t\tcover_letter_len = 1;\n+\t\t} else if (!prefixcmp(argv[i], \"--cover-letter=never\"))\n+\t\t\tcover_letter = 0;\n+\t\telse if (!prefixcmp(argv[i], \"--cover-letter=\")) {\n+\t\t\tcover_letter = 1;\n+\t\t\tcover_letter_len = strtol(argv[i] + 15, NULL, 10);\n+\t\t} else if (!strcmp(argv[i], \"--no-binary\"))\n \t\t\tno_binary_diff = 1;\n \t\telse if (!prefixcmp(argv[i], \"--add-header=\"))\n \t\t\tadd_header(argv[i] + 13);\n@@ -877,6 +891,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t}\n \targc = j;\n \n+\tif (cover_letter_len <= 0)\n+\t\tcover_letter = 0;\n+\n \tif (do_signoff) {\n \t\tconst char *committer;\n \t\tconst char *endpos;\n@@ -1010,6 +1027,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tlist[nr - 1] = commit;\n \t}\n \ttotal = nr;\n+\tif (total < cover_letter_len)\n+\t\tcover_letter = 0;\n \tif (!keep_subject && auto_number && total > 1)\n \t\tnumbered = 1;\n \tif (numbered)\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"112965","messageId":"1241431142-8444-3-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2 2/4] Add format.coverauto boolean","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T09:59:00Z","receivedAt":"2009-05-04T09:59:00Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"If set to true, format-patch behaves like it had been started\nusing the --cover-letter option.\n\nAn exception is if it is called using the --stdout option,\nwhich disables format.coverauto, because users of --stdout\n(like git-rebase.sh) usually are not interested in\ncover letters at all.\n\nIf you do want it anyway, you can use:\n % git format-patch --stdout --cover-letter\n\nSigned-off-by: Frank Terbeck \n---\n builtin-log.c | 9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 157c8cf..52c0d47 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -483,6 +483,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"diff.color\") || !strcmp(var, \"color.diff\")) {\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.coverauto\")) {\n+\t\tcover_letter = git_config_bool(var, value);;\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"format.coverletter\")) {\n \t\tcover_letter_len = git_config_int(var, value);\n \t\treturn 0;\n@@ -789,9 +793,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t * possibly a valid SHA1.\n \t */\n \tfor (i = 1, j = 1; i < argc; i++) {\n-\t\tif (!strcmp(argv[i], \"--stdout\"))\n+\t\tif (!strcmp(argv[i], \"--stdout\")) {\n \t\t\tuse_stdout = 1;\n-\t\telse if (!strcmp(argv[i], \"-n\") ||\n+\t\t\tcover_letter = 0;\n+\t\t} else if (!strcmp(argv[i], \"-n\") ||\n \t\t\t\t!strcmp(argv[i], \"--numbered\"))\n \t\t\tnumbered = 1;\n \t\telse if (!strcmp(argv[i], \"-N\") ||\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"112964","messageId":"1241431142-8444-4-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2 3/4] Add tests for coverauto, coverletter and --cover-letter","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T09:59:01Z","receivedAt":"2009-05-04T09:59:01Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Very much based on Jeff King's suggestion from\n<20090421033213.GA14881@coredump.intra.peff.net>\n\nThis change also makes sure that one major user of --stdout\n(git-rebase.sh) works with format.coverauto set to true.\n\nSigned-off-by: Frank Terbeck \n---\n t/t3400-rebase.sh | 1 +\n t/t4014-format-patch.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 57 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 6e391a3..48d8360 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -15,6 +15,7 @@ export GIT_AUTHOR_EMAIL\n test_expect_success \\\n 'prepare repository with topic branches' \\\n 'git config core.logAllRefUpdates true &&\n+ git config format.coverauto true &&\n echo First > A &&\n git update-index --add A &&\n git commit -m \"Add A.\" &&\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 11061dd..3f431ff 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -151,6 +151,60 @@ test_expect_success 'multiple files' '\n \tls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch\n '\n \n+test_expect_success '--cover-letter generates cover letter' '\n+\n+\trm -rf patches &&\n+\tgit config format.coverletter 1\n+\tgit format-patch --cover-letter -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverletter=number > 1 generates cover letter' '\n+\n+\trm -rf patches &&\n+\tgit config format.coverletter 3 &&\n+\tgit format-patch --cover-letter -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverletter respects minimum patchset size' '\n+\n+\trm -rf patches &&\n+\tgit config format.coverletter 4 &&\n+\tgit format-patch --cover-letter -o patches/ master &&\n+\t! test -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success '--cover-letter=never disables cover letters' '\n+\n+\trm -rf patches &&\n+\tgit config format.coverletter 2 &&\n+\tgit format-patch --cover-letter=never -o patches/ master &&\n+\t! test -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success '--cover-letter=always forces cover letters' '\n+\n+\trm -rf patches &&\n+\tgit format-patch --cover-letter=always -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverauto works (1)' '\n+\n+\trm -rf patches &&\n+\tgit config format.coverauto true &&\n+\tgit format-patch -1 -o patches/ master &&\n+\t! test -f patches/0000-cover-letter.patch\n+'\n+\n+test_expect_success 'format.coverauto works (2)' '\n+\n+\trm -rf patches &&\n+\tgit format-patch -2 -o patches/ master &&\n+\ttest -f patches/0000-cover-letter.patch\n+'\n+\n check_threading () {\n \texpect=\"$1\" &&\n \tshift &&\n@@ -405,9 +459,9 @@ test_expect_success 'cover-letter inherits diff options' '\n \n \tgit mv file foo &&\n \tgit commit -m foo &&\n-\tgit format-patch --cover-letter -1 &&\n+\tgit format-patch --cover-letter=always -1 &&\n \t! grep \"file => foo .* 0 *$\" 0000-cover-letter.patch &&\n-\tgit format-patch --cover-letter -1 -M &&\n+\tgit format-patch --cover-letter=always -1 -M &&\n \tgrep \"file => foo .* 0 *$\" 0000-cover-letter.patch\n \n '\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"112967","messageId":"1241431142-8444-5-git-send-email-ft@bewatermyfriend.org","threadId":"18938","inReplyTo":"7v8wlxx18c.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2 4/4] Documentation for format.coverletter and --cover-letter","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T09:59:02Z","receivedAt":"2009-05-04T09:59:02Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Signed-off-by: Frank Terbeck \n---\n Documentation/config.txt | 9 +++++++++\n Documentation/git-format-patch.txt | 17 ++++++++++++++---\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5dcad94..2842cb1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -723,6 +723,15 @@ format.attach::\n \tvalue as the boundary. See the --attach option in\n \tlinkgit:git-format-patch[1].\n \n+format.coverauto::\n+\tA boolean variable, if set linkgit:git-format-patch[1] will behave\n+\tas if it was called with its --cover-letter option, unless it was\n+\trun with its --stdout option.\n+\n+format.coverletter::\n+\tConfigures how long a patch series needs to be for cover letters\n+\tto be created. The default is 2.\n+\n format.numbered::\n \tA boolean which can enable or disable sequence numbers in patch\n \tsubjects. It defaults to \"auto\" which enables it only if there\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 6f1fc80..64f90a1 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -19,7 +19,7 @@ SYNOPSIS\n \t\t [--ignore-if-in-upstream]\n \t\t [--subject-prefix=Subject-Prefix]\n \t\t [--cc=]\n-\t\t [--cover-letter]\n+\t\t [--cover-letter[=(always|never|)]]\n \t\t []\n \t\t [ | ]\n \n@@ -107,7 +107,11 @@ include::diff-options.txt[]\n \n --stdout::\n \tPrint all commits to the standard output in mbox format,\n-\tinstead of creating a file for each one.\n+\tinstead of creating a file for each one. This also disables\n+\tthe format.coverauto option if it was set. If you really want\n+\tcover letters to be included in --stdout output you have to\n+\tspecify the --cover-letter option after the --stdout option\n+\ton the command line.\n \n --attach[=]::\n \tCreate multipart/mixed attachment, the first part of\n@@ -163,10 +167,17 @@ if that is not set.\n \tto any configured headers, and may be used multiple times.\n \tFor example, --add-header=\"Organization: git-foo\"\n \n---cover-letter::\n+--cover-letter[=(always|never|)]::\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+\tPer default, cover letters are created if a patch series is least two\n+\tpatches long (the format.coverletter allows you to change that default\n+\tlength). If a parameter is given it overwrites the default minimum\n+\tseries length. If the parameter is 0 or 'never', a cover letter will\n+\tnot be generated even if the format.coverletter option would trigger\n+\tnormally it. If the parameter is 1 or 'always' a cover letter will\n+\tbe generated even if the patch series is only one patch long.\n \n --suffix=.::\n \tInstead of using `.patch` as the suffix for generated\n-- \n1.6.2.2.446.gfbdc0\n"},{"id":"112998","messageId":"780e0a6b0905041139x3a764768ocbbd8d9353ea568f@mail.gmail.com","threadId":"18938","inReplyTo":"1241431142-8444-3-git-send-email-ft@bewatermyfriend.org","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-04T18:39:53Z","receivedAt":"2009-05-04T18:39:53Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"On Mon, May 4, 2009 at 2:59 AM, Frank Terbeck wrote:\n> An exception is if it is called using the --stdout option,\n> which disables format.coverauto, because users of --stdout\n> (like git-rebase.sh) usually are not interested in\n> cover letters at all.\n>\n\nWould it make more sense to just have git-rebase.sh use\n--cover-letter=never? I thought configuration variables were defaults\nwhich have to be overridden.\n\nAlso, why does this variable even exist? I think Jeff's suggestion is\nbest, where you can set format.coverletter to always, never, or some\nnumber.\n\n> +       if (!strcmp(var, \"format.coverauto\")) {\n> +               cover_letter = git_config_bool(var, value);;\n> +               return 0;\n> +       }\n\nDouble semi-colon?\n\nFinally, this option is very useful, so keep up the good work.\n"},{"id":"113007","messageId":"20090504214149.GA26208@fsst.voodoo.lan","threadId":"18938","inReplyTo":"780e0a6b0905041139x3a764768ocbbd8d9353ea568f@mail.gmail.com","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-04T21:41:49Z","receivedAt":"2009-05-04T21:41:49Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Stephen Boyd :\n> On Mon, May 4, 2009 at 2:59 AM, Frank Terbeck wrote:\n> > An exception is if it is called using the --stdout option,\n> > which disables format.coverauto, because users of --stdout\n> > (like git-rebase.sh) usually are not interested in\n> > cover letters at all.\n> >\n> \n> Would it make more sense to just have git-rebase.sh use\n> --cover-letter=never? I thought configuration variables were defaults\n> which have to be overridden.\n\nCould be done. And in an earlier version I did that. Since that took\nchanges in more places, I reverted to this less intrusive approach.\n\nAlso, this will keep the scripts of people who use --stdout working,\nno matter what the settings of an individual user might be.\n\nI don't think people who use --stdout will want the cover letter\n(which is always the same) in the output. Since you can still force\nits output, I think this would be reasonable compromise.\n\n> Also, why does this variable even exist? I think Jeff's suggestion is\n> best, where you can set format.coverletter to always, never, or some\n> number.\n\nWell, I did this because I wanted both Junio's and Jeff's suggestions\nto be incorporated.\n\nJunio correctly stated, that nobody will usually want cover letters\nfor one-patch \"series\". Which I think is right; and the wrapper I used\nfor this before did take that into account as well. So coverletter had\nto default to 2.\n\nIf that where to enable the generation of cover letters for all\nformat-patch calls with every patch series that is at least two\npatches long, it would change format-patch's default behaviour;\npotentially breaking people's scripts (similarly to the way\ncoverauto=true could break git-rebase.sh without either adding\n--cover-letter=never or the exception in the --stdout codepath).\n\nThat's why I think this should be handled in two separate options.\nAnd as I mentioned, coverauto could be left out if we'd advise users\nwho want that to create an git alias that does 'format-patch\n--cover-letter'.\n\n> > +\tif (!strcmp(var, \"format.coverauto\")) {\n> > +\t\tcover_letter = git_config_bool(var, value);;\n> > +\t\treturn 0;\n> > +\t}\n> \n> Double semi-colon?\n\nOops, yeah. I'll resend this one (if coverauto turns out to be the way\nto go).\n\nRegards, Frank\n\n-- \nIn protocol design, perfection has been reached not when there is\nnothing left to add, but when there is nothing left to take away.\n -- RFC 1925\n"},{"id":"113011","messageId":"7v8wlc4fqo.fsf@alter.siamese.dyndns.org","threadId":"18938","inReplyTo":"1241431142-8444-3-git-send-email-ft@bewatermyfriend.org","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-04T23:20:47Z","receivedAt":"2009-05-04T23:20:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Terbeck writes:\n\n> If set to true, format-patch behaves like it had been started\n> using the --cover-letter option.\n\nI thought \"If this is set, you can run format-patch without giving an\nexplicit --cover-letter=foo from the command line\" was already done with\nthe earlier format.coverletter configuration variable. Why do you need a\nseparate variable? It does not make any sense to me, unless I am missing\nsomething.\n"},{"id":"113025","messageId":"20090505084916.GB26208@fsst.voodoo.lan","threadId":"18938","inReplyTo":"7v8wlc4fqo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-05T08:49:16Z","receivedAt":"2009-05-05T08:49:16Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Junio C Hamano :\n> Frank Terbeck writes:\n> \n> > If set to true, format-patch behaves like it had been started\n> > using the --cover-letter option.\n> \n> I thought \"If this is set, you can run format-patch without giving an\n> explicit --cover-letter=foo from the command line\" was already done with\n> the earlier format.coverletter configuration variable. Why do you need a\n> separate variable? It does not make any sense to me, unless I am missing\n> something.\n\nWell, the two can certainly by merged. That could potentially break\npeople's existing scripts - either by new default behaviour or by the\nsetting of format.coverletter of an individual user. That could still\nhappen when using coverauto, so maybe my reasoning was flawed - given\nthat Stephen raised the same question.\n\nSo, I should create one option 'coverletter'. If it's set to zero,\nnever create cover letters; if one, always create cover letters; if\nany other positive integer, create cover letters automatically if a\npatch series is at least that long. And do that without requiring the\nuser to supply --cover-letter; only provide that option to explicitly\noverwrite the configured behaviour. Right?\n\nIf so, do you want coverletter to default to zero (which wouldn't\nchange the default behaviour) or do you want it to default to two?\n\nRegards, Frank\n\n-- \nIn protocol design, perfection has been reached not when there is\nnothing left to add, but when there is nothing left to take away.\n -- RFC 1925\n"},{"id":"113030","messageId":"7vvdof25oe.fsf@alter.siamese.dyndns.org","threadId":"18938","inReplyTo":"20090505084916.GB26208@fsst.voodoo.lan","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-05T10:41:05Z","receivedAt":"2009-05-05T10:41:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frank Terbeck writes:\n\n> ... That could potentially break\n> people's existing scripts - either by new default behaviour or by the\n> setting of format.coverletter of an individual user. That could still\n> happen when using coverauto, so maybe my reasoning was flawed - given\n> that Stephen raised the same question.\n\nThat part I agree with 100%. A separate configuration variable does not\nhelp here; it will not act as an \"I know what I am doing\" flag.\n\n\"rebase\" is not a problem because both format-patch and rebase come from\nthe same git and people do not mix and match. The only thing we need to\ndo is to make sure that rebase is updated to explicitly decline the cover\nletter in the same or earlier version of git that adds this new option.\n\nHowever, there are other people's scripts to worry about, and no matter\nwhat you do, you cannot avoid breaking them if you introduce a\nconfiguration variable that changes the behaviour of the command in a\nbackward incompatible way. This could be a disruptive change that needs\nto happen at a major revision boundary, if we were to add it.\n\n> If so, do you want coverletter to default to zero (which wouldn't\n> change the default behaviour) or do you want it to default to two?\n\nThe default between 0 and 2 does not matter much. If people find this new\nfeature useful (if not, then this new feature is not worth adding), then\nthey will set it to non-zero themselves, and then get their scripts\nbroken. In a sense, defaulting it to zero is just delaying an inevitable\nbreakage.\n\nIf we were to do this, we should rather default it to two from day one and\nmake sure we break people's scripts that (rightly) rely on format-patch\nnot producing covers that the caller never asked, to make sure they are\nadjusted to decline cover explicitly. And such a release should come with\na note in huge red letters that says \"this new feature allows you not to\nsay --cover on the command line, and it is far more important than keeping\nyour scripts that run format-patch and process its output working. You\nmust adjust your scripts to the new reality. Sorry for the inconvenience,\nand have a nice day.\"\n\nI do not think I would stand behind such a release note, though.\n\nFor one thing, I do not see a huge need for this configuration variable.\nWhy is \"--cover\" too cumbersome to type, when you are already willing to\ntype \"format-patch\"? You can alias the whole thing away to make it even\nshorter. For another, we do not simply break people's scripts knowingly.\n\nFor this feature to go forward, somebody has to find a way not to break\npeople's scripts even when the user uses it. One possibility is to find a\nnicer verb X and introduce \"git X\" command that does what \"format-patch\"\ndoes but with a better default (and syntax --- people get confused by the\noldest form \"git format-patch $commit\", which does not behave like \"git\nlog $commit\" simply because the syntax predates the modern \"revision\nrange\" notation the \"log\" family supports, such as A..B).\n"},{"id":"113037","messageId":"20090505132923.GC26208@fsst.voodoo.lan","threadId":"18938","inReplyTo":"7vvdof25oe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-05T13:29:24Z","receivedAt":"2009-05-05T13:29:24Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Junio C Hamano :\n[...]\n> For one thing, I do not see a huge need for this configuration variable.\n> Why is \"--cover\" too cumbersome to type, when you are already willing to\n> type \"format-patch\"? You can alias the whole thing away to make it even\n> shorter. For another, we do not simply break people's scripts knowingly.\n> \n> For this feature to go forward, somebody has to find a way not to break\n> people's scripts even when the user uses it. One possibility is to find a\n> nicer verb X and introduce \"git X\" command that does what \"format-patch\"\n> does but with a better default (and syntax --- people get confused by the\n> oldest form \"git format-patch $commit\", which does not behave like \"git\n> log $commit\" simply because the syntax predates the modern \"revision\n> range\" notation the \"log\" family supports, such as A..B).\n\nYes, this whole compatibility issue is exactly the reason why I said\nearlier, that we could only take format.coverletter (the way it's\ncurrently implemented in the latest series) and forget about\nformat.coverauto (and its implications) altogether:\n\n<1241431142-8444-1-git-send-email-ft@bewatermyfriend.org>:\n} Now that I think of it again, two weeks after writing this mail\n} originally, I guess it would be possible to drop format.coverauto\n} altogether and tell users to use:\n} \n} % git config --global alias.fp format-patch --cover-letter\n} \n} I don't know which solution would be preferred. If it's the user-should-\n} use-an-alias approach, I'll create a series that gets rid of\n} format.coverauto changes.\n\nI obviously was missing quotes around the alias's value but then,\n'git fp' would do exactly what I'm after with this series. You could\njust set format.coverletter to 2 and use 'git fp'. You always get a\ncover letter for patch series longer than one patch.\n\nThe other approach, to create yet another subcommand just for this\nwould be too much IMHO. There are enough of them already.\n\nRegards, Frank\n\n-- \nIn protocol design, perfection has been reached not when there is\nnothing left to add, but when there is nothing left to take away.\n -- RFC 1925\n"},{"id":"113043","messageId":"20090505152338.GD26208@fsst.voodoo.lan","threadId":"18938","inReplyTo":"20090505132923.GC26208@fsst.voodoo.lan","subject":"Re: [PATCH v2 2/4] Add format.coverauto boolean","fromName":"Frank Terbeck","fromEmail":"ft@bewatermyfriend.org","sentAt":"2009-05-05T15:23:38Z","receivedAt":"2009-05-05T15:23:38Z","isPatch":true,"sender":{"key":"ft@bewatermyfriend.org","avatar":null},"body":"Frank Terbeck :\n[...]\n> } % git config --global alias.fp format-patch --cover-letter\n\nThat may still break backwards compatibility.\nTo be safe, we could make --cover-letter behave as it always did, and\nonly make it look at format.coverletter if --cover-letter=auto was\nspecified.\n\nRegards, Frank\n"}]}