{"thread":{"id":"19358","subject":"[RFC PATCH] builtin-log: Add options to --coverletter","startedAt":"2009-05-15T00:57:21Z","lastAt":"2009-05-16T17:35:46Z","messageCount":10,"participants":["Joe Perches","Junio C Hamano","Jeff King","James Cloos"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"113979","messageId":"1242349041.646.8.camel@Joe-Laptop.home","threadId":"19358","inReplyTo":null,"subject":"[RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2009-05-15T00:57:21Z","receivedAt":"2009-05-15T00:57:21Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"Currently the coverletter options for wrapping long lines\nin the shortlog are: on, wrap as position72, with fixed indents.\n\nI think these defaults can produce poor looking output.\n\nThis patch allows these to be optionally specified on the\ncommand line with --cover-letter[=wrap[,pos[,in1[,in2]]]]\n\nI'm not sure this is the right approach though.\n\nSuggestions?\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..de26c04 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -460,6 +460,11 @@ static void add_header(const char *value)\n static int thread = 0;\n static int do_signoff = 0;\n \n+static int coverletter_wrap = 1;\n+static int coverletter_wraplen = 72;\n+static int coverletter_indent1 = 2;\n+static int coverletter_indent2 = 4;\n+\n static int git_format_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"format.headers\")) {\n@@ -668,10 +673,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tstrbuf_release(&sb);\n \n \tshortlog_init(&log);\n-\tlog.wrap_lines = 1;\n-\tlog.wrap = 72;\n-\tlog.in1 = 2;\n-\tlog.in2 = 4;\n+\tlog.wrap_lines = coverletter_wrap;\n+\tlog.wrap = coverletter_wraplen;\n+\tlog.in1 = coverletter_indent1;\n+\tlog.in2 = coverletter_indent2;\n \tfor (i = 0; i < nr; i++)\n \t\tshortlog_add_commit(&log, list[i]);\n \n@@ -866,8 +871,17 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\trev.subject_prefix = argv[i] + 17;\n \t\t} else if (!prefixcmp(argv[i], \"--suffix=\"))\n \t\t\tfmt_patch_suffix = argv[i] + 9;\n-\t\telse if (!strcmp(argv[i], \"--cover-letter\"))\n+\t\telse if (!prefixcmp(argv[i], \"--cover-letter\")) {\n \t\t\tcover_letter = 1;\n+\t\t\tif (*(argv[i] + 14) == '=') {\n+\t\t\t\tif (sscanf(argv[i] + 15, \"%d,%d,%d,%d\",\n+\t\t\t\t\t   &coverletter_wrap,\n+\t\t\t\t\t   &coverletter_wraplen,\n+\t\t\t\t\t   &coverletter_indent1,\n+\t\t\t\t\t   &coverletter_indent2) <= 0)\n+\t\t\t\t\tdie(\"Need options for --cover-letter=\");\n+\t\t\t}\n+\t\t}\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"},{"id":"114009","messageId":"7v63g2tewu.fsf@alter.siamese.dyndns.org","threadId":"19358","inReplyTo":"1242349041.646.8.camel@Joe-Laptop.home","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-15T18:11:13Z","receivedAt":"2009-05-15T18:11:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n> Currently the coverletter options for wrapping long lines\n> in the shortlog are: on, wrap as position72, with fixed indents.\n>\n> I think these defaults can produce poor looking output.\n>\n> This patch allows these to be optionally specified on the\n> command line with --cover-letter[=wrap[,pos[,in1[,in2]]]]\n\nI think it makes sense to let users affect how the short-log in the cover\nletter is generated.  I do not think overloading the --cover-letter option\nfor doing it is the ideal approach, though.\n\n - Currently we never generate a cover without being asked, but if we ever\n   switch that default in the future, we would want to be able to decline\n   it from the command line with --cover-letter=no (or --no-cover-letter).\n   The \"no\" here is different from \"please do not wrap but do produce\n   cover letter\".\n\n - Currently there is only one style of cover letter, but people may want\n   to add a mechanism to let them use different styles that suit their\n   project better in the future, and a natural syntax to ask for a\n   different style is --cover-letter=style, where \"style\" may be\n   \"default\", \"shortlog\", or some other token that the user invents.\n   --cover-letter=no will still ask to suppress cover letters.\n\nThis is a tangent, but I do not think the current cover-letter that uses\nshortlog matches everybody's needs.  The shortlog format lists commits\ngrouped by the author and does not number them, and it makes it hard to\nmatch which message in the series corresponds to which entry in the cover\nletter, especially when your series have a resend of somebody else's patch\nin it.  I wouldn't be surprised if somebody comes up with a different\nstyle that is based on \"git log --reverse --oneline A..B\" output (perhaps\nwithout the shortened object name part) and name it the \"oneline\" style,\ne.g.\n\n    From: Jeff King\n\n    *** BLURB HERE ***\n    The following patches do ...\n\n    1/2\tparseopt: add OPT_NEGBIT (Réne Scharfe)\n    2/2 ls-files: make --no-empty-directory negatable\n\n     Documentation/technical/api-parse-options.txt |    4 +++\n     ...\n     test-parse-options.c                          |    1 +\n     6 files changed, 45 insertions(+), 3 deletions(-)\n\n\nNow, where does \"line wrapping parameters\" fit in the picture of this\npossible future with multiple styles?  From the implementation convenience\nviewpoint, you could make the line wrapping knobs specific to the\n\"shortlog\" style.  It however is conceivable that \"oneline\" style (yet to\nbe written by somebody) may want to wrap its output, and the parameters it\nwould want to use may well be the same set.\n\nI think it would make sense to introduce a separate parameter:\n\n\t--cover-letter-wrap=<pos>,<indent>,<wrap-offset>\n\nat this point in your patch.  It does not add oneline or any other\ndifferent styles, but it would keep the door open for later additions\nwithout breaking the UI.\n\nBy the way, don't people find the semantics of in1/in2 to shortlog\nwrapping unnatural?  By the above three-tuple parameter, I meant:\n\n\twrap at position [pos], indent the whole thing by this much\n\t[indent], and offset the second and subsequent lines by this much\n\t[wrap-offset].\n\nand that reads much better than the -w option of shortlog that says\n\n\twrap at position [pos], indent the whole thing by this much\n\t[in1], and offset the second and subsequent lines by this much\n\t[in2 minus in1].\n"},{"id":"114029","messageId":"1242418762.3373.90.camel@Joe-Laptop.home","threadId":"19358","inReplyTo":"7v63g2tewu.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2009-05-15T20:19:22Z","receivedAt":"2009-05-15T20:19:22Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Fri, 2009-05-15 at 11:11 -0700, Junio C Hamano wrote:\n> I think it makes sense to let users affect how the short-log in the cover\n> letter is generated.  I do not think overloading the --cover-letter option\n> for doing it is the ideal approach, though.\n\nOK.  How about this patch?\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..49fd42a 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -460,6 +460,11 @@ static void add_header(const char *value)\n static int thread = 0;\n static int do_signoff = 0;\n \n+static int coverletter_wrap = 1;\n+static int coverletter_wrappos = 72;\n+static int coverletter_indent1 = 2;\n+static int coverletter_indent2 = 4;\n+\n static int git_format_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"format.headers\")) {\n@@ -668,10 +673,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tstrbuf_release(&sb);\n \n \tshortlog_init(&log);\n-\tlog.wrap_lines = 1;\n-\tlog.wrap = 72;\n-\tlog.in1 = 2;\n-\tlog.in2 = 4;\n+\tlog.wrap_lines = coverletter_wrap;\n+\tlog.wrap = coverletter_wrappos;\n+\tlog.in1 = coverletter_indent1;\n+\tlog.in2 = coverletter_indent2;\n \tfor (i = 0; i < nr; i++)\n \t\tshortlog_add_commit(&log, list[i]);\n \n@@ -868,6 +873,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 (!prefixcmp(argv[i], \"--cover-letter-wrap=\")) {\n+\t\t\tif (sscanf(argv[i] + 20, \"%d,%d,%d\",\n+\t\t\t\t   &coverletter_wrappos,\n+\t\t\t\t   &coverletter_indent1,\n+\t\t\t\t   &coverletter_indent2) <= 0)\n+\t\t\t\tdie(\"Need options for --cover-letter-wrap=\");\n+\t\t\tif (coverletter_wrappos == 0)\n+\t\t\t\tcoverletter_wrap = 0;\n+\t\t\t}\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"},{"id":"114040","messageId":"7vljoyrq4z.fsf@alter.siamese.dyndns.org","threadId":"19358","inReplyTo":"1242418762.3373.90.camel@Joe-Laptop.home","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-15T21:51:40Z","receivedAt":"2009-05-15T21:51:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n> On Fri, 2009-05-15 at 11:11 -0700, Junio C Hamano wrote:\n>> I think it makes sense to let users affect how the short-log in the cover\n>> letter is generated.  I do not think overloading the --cover-letter option\n>> for doing it is the ideal approach, though.\n>\n> OK.  How about this patch?\n\nI'd suggest...\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 5eaec5d..49fd42a 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -460,6 +460,11 @@ static void add_header(const char *value)\n>  static int thread = 0;\n>  static int do_signoff = 0;\n>  \n> +static int coverletter_wrap = 1;\n\nDo not change the default behaviour before people agree it is a good\nfeature;\n\n\tstatic int coverletter_wrap;\n\n> +static int coverletter_wrappos = 72;\n> +static int coverletter_indent1 = 2;\n> +static int coverletter_indent2 = 4;\n> +\n>  static int git_format_config(const char *var, const char *value, void *cb)\n>  {\n>  \tif (!strcmp(var, \"format.headers\")) {\n> @@ -668,10 +673,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n>  \tstrbuf_release(&sb);\n>  \n>  \tshortlog_init(&log);\n> -\tlog.wrap_lines = 1;\n> -\tlog.wrap = 72;\n> -\tlog.in1 = 2;\n> -\tlog.in2 = 4;\n> +\tlog.wrap_lines = coverletter_wrap;\n> +\tlog.wrap = coverletter_wrappos;\n> +\tlog.in1 = coverletter_indent1;\n> +\tlog.in2 = coverletter_indent2;\n>  \tfor (i = 0; i < nr; i++)\n>  \t\tshortlog_add_commit(&log, list[i]);\n>  \n> @@ -868,6 +873,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 (!prefixcmp(argv[i], \"--cover-letter-wrap=\")) {\n> +\t\t\tif (sscanf(argv[i] + 20, \"%d,%d,%d\",\n> +\t\t\t\t   &coverletter_wrappos,\n> +\t\t\t\t   &coverletter_indent1,\n> +\t\t\t\t   &coverletter_indent2) <= 0)\n> +\t\t\t\tdie(\"Need options for --cover-letter-wrap=\");\n> +\t\t\tif (coverletter_wrappos == 0)\n> +\t\t\t\tcoverletter_wrap = 0;\n\n... lose this \"if ()\"; if you are asking for --cover-letter-wrap from the\ncommand line explicitly, you do want the result to be wrapped.\n\nIn order to prepare yourself for change of default in the future (or\nadding configurable defaults), the command line parser (the sscanf()\nabove) needs to understand something like \"--cover-letter-linewrap=no\", in\naddition to the up-to-three integers it currently takes via sscanf().\nTreating \"the resulting line should be wrapped at 0 column\" as \"please do\nnot wrap\" may work in practice but I do not think it is a good style.\n"},{"id":"114042","messageId":"1242425263.31337.17.camel@Joe-Laptop.home","threadId":"19358","inReplyTo":"7vljoyrq4z.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2009-05-15T22:07:43Z","receivedAt":"2009-05-15T22:07:43Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Fri, 2009-05-15 at 14:51 -0700, Junio C Hamano wrote:\n> Joe Perches <joe@perches.com> writes:\n> \n> > On Fri, 2009-05-15 at 11:11 -0700, Junio C Hamano wrote:\n> >> I think it makes sense to let users affect how the short-log in the cover\n> >> letter is generated.  I do not think overloading the --cover-letter option\n> >> for doing it is the ideal approach, though.\n> >\n> > OK.  How about this patch?\n> \n> I'd suggest...\n> \n> > diff --git a/builtin-log.c b/builtin-log.c\n> > index 5eaec5d..49fd42a 100644\n> > --- a/builtin-log.c\n> > +++ b/builtin-log.c\n> > @@ -460,6 +460,11 @@ static void add_header(const char *value)\n> >  static int thread = 0;\n> >  static int do_signoff = 0;\n> >  \n> > +static int coverletter_wrap = 1;\n> \n> Do not change the default behaviour before people agree it is a good\n> feature;\n\nThis doesn't change the default behavior.\nThe default is still wrap enabled.\n\n> \tstatic int coverletter_wrap;\n> \n> > +static int coverletter_wrappos = 72;\n> > +static int coverletter_indent1 = 2;\n> > +static int coverletter_indent2 = 4;\n> > +\n> >  static int git_format_config(const char *var, const char *value, void *cb)\n> >  {\n> >  \tif (!strcmp(var, \"format.headers\")) {\n> > @@ -668,10 +673,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n> >  \tstrbuf_release(&sb);\n> >  \n> >  \tshortlog_init(&log);\n> > -\tlog.wrap_lines = 1;\n> > -\tlog.wrap = 72;\n> > -\tlog.in1 = 2;\n> > -\tlog.in2 = 4;\n> > +\tlog.wrap_lines = coverletter_wrap;\n> > +\tlog.wrap = coverletter_wrappos;\n> > +\tlog.in1 = coverletter_indent1;\n> > +\tlog.in2 = coverletter_indent2;\n> >  \tfor (i = 0; i < nr; i++)\n> >  \t\tshortlog_add_commit(&log, list[i]);\n> >  \n> > @@ -868,6 +873,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 (!prefixcmp(argv[i], \"--cover-letter-wrap=\")) {\n> > +\t\t\tif (sscanf(argv[i] + 20, \"%d,%d,%d\",\n> > +\t\t\t\t   &coverletter_wrappos,\n> > +\t\t\t\t   &coverletter_indent1,\n> > +\t\t\t\t   &coverletter_indent2) <= 0)\n> > +\t\t\t\tdie(\"Need options for --cover-letter-wrap=\");\n> > +\t\t\tif (coverletter_wrappos == 0)\n> > +\t\t\t\tcoverletter_wrap = 0;\n> \n> ... lose this \"if ()\"; if you are asking for --cover-letter-wrap from the\n> command line explicitly, you do want the result to be wrapped.\n[]\n> In order to prepare yourself for change of default in the future (or\n> adding configurable defaults), the command line parser (the sscanf()\n> above) needs to understand something like \"--cover-letter-linewrap=no\", in\n> addition to the up-to-three integers it currently takes via sscanf().\n> Treating \"the resulting line should be wrapped at 0 column\" as \"please do\n> not wrap\" may work in practice but I do not think it is a good style.\n\nPrefixing \"no-\" to git arguments seems widely used.\n\nPerhaps:\n  --no-cover-letter-wrap\nand\n  --cover-letter-wrap=pos[,indent1[,indent2]]\n"},{"id":"114046","messageId":"1242434796.4070.2.camel@Joe-Laptop.home","threadId":"19358","inReplyTo":"7vljoyrq4z.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2009-05-16T00:46:36Z","receivedAt":"2009-05-16T00:46:36Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"Perhaps this?\n\nSigned-off-by: Joe Perches <joe@perches.com>\n---\n Documentation/git-format-patch.txt     |   13 +++++++++++++\n builtin-log.c                          |   22 ++++++++++++++++++----\n contrib/completion/git-completion.bash |    1 +\n 3 files changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 6f1fc80..f6b34ff 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,6 +20,8 @@ SYNOPSIS\n \t\t   [--subject-prefix=Subject-Prefix]\n \t\t   [--cc=<email>]\n \t\t   [--cover-letter]\n+\t\t   [--cover-letter-wrap=width[,indent1[,indent2]]]\n+\t\t   [--no-cover-letter-wrap]\n \t\t   [<common diff options>]\n \t\t   [ <since> | <revision range> ]\n \n@@ -168,6 +170,17 @@ 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-letter-wrap=<width>[,<indent1>[,<indent2>]]]::\n+\tLinewrap the cover-letter shortlog output by wrapping each line at\n+\t`width`.  The first line of each entry is indented by `indent1`\n+\tspaces, and the second and subsequent lines are indented by\n+\t`indent2` spaces.\n+\t`width`, `indent1`, and `indent2` default to 72, 2 and 4 respectively.\n+\n+--no-cover-letter-wrap::\n+\tDo not linewrap the cover-letter shortlog output.\n+\tindent is fixed at 6.\n+\n --suffix=.<sfx>::\n \tInstead of using `.patch` as the suffix for generated\n \tfilenames, use specified suffix.  A common alternative is\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..271cbc1 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -460,6 +460,11 @@ static void add_header(const char *value)\n static int thread = 0;\n static int do_signoff = 0;\n \n+static int cover_letter_wrap = 1;\n+static int cover_letter_wrappos = 72;\n+static int cover_letter_indent1 = 2;\n+static int cover_letter_indent2 = 4;\n+\n static int git_format_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"format.headers\")) {\n@@ -668,10 +673,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tstrbuf_release(&sb);\n \n \tshortlog_init(&log);\n-\tlog.wrap_lines = 1;\n-\tlog.wrap = 72;\n-\tlog.in1 = 2;\n-\tlog.in2 = 4;\n+\tlog.wrap_lines = cover_letter_wrap;\n+\tlog.wrap = cover_letter_wrappos;\n+\tlog.in1 = cover_letter_indent1;\n+\tlog.in2 = cover_letter_indent2;\n \tfor (i = 0; i < nr; i++)\n \t\tshortlog_add_commit(&log, list[i]);\n \n@@ -868,6 +873,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-cover-letter-wrap\"))\n+\t\t\tcover_letter_wrap = 0;\n+\t\telse if (!prefixcmp(argv[i], \"--cover-letter-wrap=\")) {\n+\t\t\tif (sscanf(argv[i] + 20, \"%d,%d,%d\",\n+\t\t\t\t   &cover_letter_wrappos,\n+\t\t\t\t   &cover_letter_indent1,\n+\t\t\t\t   &cover_letter_indent2) <= 0)\n+\t\t\t\tdie(\"Need options for --cover-letter-wrap=\");\n+\t\t\t}\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=\"))\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex ad26b7c..2f5c42b 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -969,6 +969,7 @@ _git_format_patch ()\n \t\t\t--full-index --binary\n \t\t\t--not --all\n \t\t\t--cover-letter\n+\t\t\t--no-cover-letter-wrap --cover-letter-wrap=\n \t\t\t--no-prefix --src-prefix= --dst-prefix=\n \t\t\t--inline --suffix= --ignore-if-in-upstream\n \t\t\t--subject-prefix=\n-- \n1.6.3.1.9.g95405b.dirty\n"},{"id":"114056","messageId":"7v63g1srsq.fsf@alter.siamese.dyndns.org","threadId":"19358","inReplyTo":"1242425263.31337.17.camel@Joe-Laptop.home","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-16T02:30:29Z","receivedAt":"2009-05-16T02:30:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n>> > +static int coverletter_wrap = 1;\n>> \n>> Do not change the default behaviour before people agree it is a good\n>> feature;\n>\n> This doesn't change the default behavior.\n> The default is still wrap enabled.\n\nYou are correct; my mistake.\n\n> Prefixing \"no-\" to git arguments seems widely used.\n>\n> Perhaps:\n>   --no-cover-letter-wrap\n\nAgain you are right; it looks much better.\n"},{"id":"114057","messageId":"7vy6sxrd5d.fsf@alter.siamese.dyndns.org","threadId":"19358","inReplyTo":"1242434796.4070.2.camel@Joe-Laptop.home","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-16T02:32:14Z","receivedAt":"2009-05-16T02:32:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n> Perhaps this?\n\nYup, but that comes after the --- we see below ;-).\n\n> Signed-off-by: Joe Perches <joe@perches.com>\n> ---\n\nThe patch looks good.\n"},{"id":"114072","messageId":"20090516050718.GA7330@sigio.peff.net","threadId":"19358","inReplyTo":"7v63g2tewu.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-05-16T05:07:19Z","receivedAt":"2009-05-16T05:07:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 15, 2009 at 11:11:13AM -0700, Junio C Hamano wrote:\n\n> This is a tangent, but I do not think the current cover-letter that uses\n> shortlog matches everybody's needs.  The shortlog format lists commits\n> grouped by the author and does not number them, and it makes it hard to\n> match which message in the series corresponds to which entry in the cover\n> letter, especially when your series have a resend of somebody else's patch\n> in it.  I wouldn't be surprised if somebody comes up with a different\n> style that is based on \"git log --reverse --oneline A..B\" output (perhaps\n> without the shortened object name part) and name it the \"oneline\" style,\n> e.g.\n> \n>     From: Jeff King\n> \n>     *** BLURB HERE ***\n>     The following patches do ...\n> \n>     1/2\tparseopt: add OPT_NEGBIT (Réne Scharfe)\n>     2/2 ls-files: make --no-empty-directory negatable\n\nAt one point I was working on --pretty=format specifiers for \"total\nnumber of commits\" and \"incremental commit number\". The eventual goal\nbeing an option like coverletter.logformat that you could set to\n\"%xi/%xn %s\".\n\nSadly, the code got a bit messy because the feature straddles the line\nof pretty.c and actual rev traversal. I started some refactoring, but\ndropped it halfway through, and now of course it is woefully out of date\n(a lesson in \"merge early, merge often\"). So I just pipe \"git log\n--oneline\" through nl manually. ;)\n\nSo yes, I think somebody would be interested in alternate styles. And\nwhile I think most people would want to set their default style as a\nconfig variable, it may make sense to override on the command-line\n(e.g., for a series that is mostly from you versus one that is from\nmixed authors).\n\n-Peff\n"},{"id":"114107","messageId":"m3my9dhrw5.fsf@lugabout.jhcloos.org","threadId":"19358","inReplyTo":"1242349041.646.8.camel@Joe-Laptop.home","subject":"Re: [RFC PATCH] builtin-log: Add options to --coverletter","fromName":"James Cloos","fromEmail":"cloos@jhcloos.com","sentAt":"2009-05-16T17:35:46Z","receivedAt":"2009-05-16T17:35:46Z","isPatch":true,"sender":{"key":"cloos@jhcloos.com","avatar":"https://gravatar.com/avatar/ec9a05787d29afe41e243e4b60bd0e2f69d757688e8f0bfe5e78bc185a3e317f?d=mp&s=160"},"body":">>>>> \"Joe\" == Joe Perches <joe@perches.com> writes:\n\nJoe> Currently the coverletter options for wrapping long lines\nJoe> in the shortlog are: on, wrap as position72, with fixed indents.\n\nJoe> I think these defaults can produce poor looking output.\n\nJoe> This patch allows these to be optionally specified on the\nJoe> command line with --cover-letter[=wrap[,pos[,in1[,in2]]]]\n\nJoe> I'm not sure this is the right approach though.\n\nIf one wants to be really cool, one could borrow the relevant code from\nGNU fmt(1).  (Be sure to grab from a release before the switch to GPL3.)\n\nGNU fmt(1) uses an algorithm based on TeX's and produces *much* better\nresults than anything else I've seen.\n\nIt is currently distributed as part of coreutils.\n\n-JimC\n-- \nJames Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6\n"}]}