{"thread":{"id":"46263","subject":"[PATCH/RFC] commit-template: improve readability of commit template","startedAt":"2017-06-26T17:24:54Z","lastAt":"2017-07-09T18:55:03Z","messageCount":25,"participants":["Kaartic Sivaraam","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"323276","messageId":"20170626172433.19785-1-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":null,"subject":"[PATCH/RFC] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-26T17:24:33Z","receivedAt":"2017-06-26T17:24:54Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"* Previously the commit template didn't separate the\n  distinct messages shown. This resulted in difficulty\n  in interpreting it's content. Add new lines to separate\n  the distinct parts of the template.\n\n* Previously the warning about usage of explicit paths\n  without any options wasn't clear. Make it more clear\n  so user gets what it's trying to say.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n I've tried to improve the message specified in the commit. Hope\n it works correctly.\n\n Local test passed.\n\n builtin/commit.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8d1cac062..0a5676b76 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -841,9 +841,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n \t\t\t\t  \" yourself if you want to.\\n\"\n \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n-\t\tif (only_include_assumed)\n+\t\tif (only_include_assumed) {\n+\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); // Add new line for clarity\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n \t\t\t\t\t\"%s\", only_include_assumed);\n+\t\t}\n \n \t\t/*\n \t\t * These should never fail because they come from our own\n@@ -877,8 +879,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n \n-\t\tif (ident_shown)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n+\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); // Add new line for clarity\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n@@ -1209,7 +1210,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tif (argc > 0 && !also && !only)\n-\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n+\t\tonly_include_assumed = _(\"Explicit paths (<paths>) specified without -i or -o; assuming --only <paths>\");\n \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n-- \n2.11.0\n\n"},{"id":"323294","messageId":"xmqqefu64dgz.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170626172433.19785-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH/RFC] commit-template: improve readability of commit template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-26T21:59:40Z","receivedAt":"2017-06-26T21:59:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> * Previously the commit template didn't separate the\n>   distinct messages shown. This resulted in difficulty\n>   in interpreting it's content. Add new lines to separate\n>   the distinct parts of the template.\n>\n> * Previously the warning about usage of explicit paths\n>   without any options wasn't clear. Make it more clear\n>   so user gets what it's trying to say.\n>\n\nWe don't usually make a bullet list in log message.  Please stick to\na plain prose.  \n\n\"Previously\" is superflous.  Say what it does (e.g. \"The commit\ntemplate adds optional parts without extra blank lines to its normal\noutput\") in present tense and explain the ramifications of it\n(e.g. \"I personally find that this makes it harder to find the\noptional bit\").\n\n> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n> ---\n>  I've tried to improve the message specified in the commit. Hope\n>  it works correctly.\n>\n>  Local test passed.\n\nPerhaps you would want to ensure that this change (if you find it\nvaluable) will not get broken by other people in the future by\nwriting a new test that ensures that these extra blank lines are\nalways there when you think they are needed?\n\nI personally do not find these new blank lines are necessary, and\nthis change wastes vertical screen real estate which is a limited\nresource, but that may be just me.  I on the other hand do not think\nthe result of this patch is overly worse than the status quo, either.\n\n\n\n>  builtin/commit.c | 9 +++++----\n>  1 file changed, 5 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 8d1cac062..0a5676b76 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -841,9 +841,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n>  \t\t\t\t  \" yourself if you want to.\\n\"\n>  \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n> -\t\tif (only_include_assumed)\n> +\t\tif (only_include_assumed) {\n> +\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); // Add new line for clarity\n>  \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n>  \t\t\t\t\t\"%s\", only_include_assumed);\n> +\t\t}\n\nWe do not use // comment in most parts of our codebase that are\nsupposed to be platform neutral (iow, compat/ is exempt).\n\nBut more importantly, wouldn't\n\n\t\tif (only_include_assumed)\n\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t\t\"%s\", only_include_asssumed);\n+\t\t\t\t\t\"\\n%s\", only_include_asssumed);\n\nbe sufficient?\n\n> @@ -877,8 +879,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n>  \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n>  \n> -\t\tif (ident_shown)\n> -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n> +\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); // Add new line for clarity\n\nThis does ensure that an extra blank line appears after the optional\nsection (either after the \"only/include assumed\" message, or \"writing\nfor somebody else\" message).\n\nIf we were to go with this sparser output, I think we also should\ngive an extra blank line before and after the \"HEAD detached from\ncafebabe\" message you would see:\n\n\t$ git checkout HEAD^0\n\t$ git commit --allow-empty -o\n\nor \"On branch blah\" if you are on a branch.  I think your change\nadds a blank before, but it does not have a separation before\n\"Changes not staged for commit\" line.\n\n>  \t\tsaved_color_setting = s->use_color;\n>  \t\ts->use_color = 0;\n> @@ -1209,7 +1210,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>  \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n>  \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n>  \tif (argc > 0 && !also && !only)\n> -\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n> +\t\tonly_include_assumed = _(\"Explicit paths (<paths>) specified without -i or -o; assuming --only <paths>\");\n\nI think \"paths (<paths>)\" is excessive.  If you are using <token> to\nhint that they refer to \"commit -h\" or \"commit --help\" output, then\n\n    Explicit <paths> specified without -i or -o; assumign --only <paths>\n\nshould be sufficient.\n\nHaving said that, to be quite honest, I think this \"assuming --only\"\nmessage outlived its usefulness.  This was necessary in very early\ndays of Git because originally \"git commit foo\" did \"git add foo &&\ngit commit\" (i.e. \"-i\" was the default) and then later when we made\n\"--only\" the new default in order to match everybody else's SCM, we\nneeded to remind users of older versions of Git that \"git commit foo\"\nnow means \"git commit --only foo\", not \"git commit -i foo\" which they\nmay have been used to.  These days, hopefully nobody expects the \"-i\"\nsemantics when they do \"git commit foo\", so perhaps it may be a better\nchange to _remove_ the message altogether.\n\nAnd with that done, I wouldn't have reservations on this change\n(i.e. \"is it worth wasting extra screen real estate, especially in\nthe vertical direction?\"), as instead of wasting 2 lines to give a\nmessage that is no longer useful in today's world, it will be\nremoving one line ;-)\n\nThanks.\n"},{"id":"323353","messageId":"1498584144.2737.7.camel@gmail.com","threadId":"46263","inReplyTo":"xmqqefu64dgz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-27T17:22:24Z","receivedAt":"2017-06-27T17:22:35Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Mon, 2017-06-26 at 14:59 -0700, Junio C Hamano wrote:\n> We don't usually make a bullet list in log message.  Please stick to\n> a plain prose.  \n> \n> \"Previously\" is superflous.  Say what it does (e.g. \"The commit\n> template adds optional parts without extra blank lines to its normal\n> output\") in present tense and explain the ramifications of it\n> (e.g. \"I personally find that this makes it harder to find the\n> optional bit\").\n> \nCorrected it.\n\n> Perhaps you would want to ensure that this change (if you find it\n> valuable) will not get broken by other people in the future by\n> writing a new test that ensures that these extra blank lines are\n> always there when you think they are needed?\n> \nSee comment below.\n\n> I personally do not find these new blank lines are necessary, and\n> this change wastes vertical screen real estate which is a limited\n> resource, but that may be just me.  I on the other hand do not think\n> the result of this patch is overly worse than the status quo, either.\n> \nI thought it's not good to trade-off readability for vertical space as\nthe ultimate aim of the commit template (at least to me) is to convey\ninformation to the user about the commit that he's going to make. For\nwhich, I thought it made more sense to improve it's readability by\nadding new lines between different sections rather than constrain the\noutput within a few lines.\n\nIn case it's not worth it, I'll revert it back which isn't that big an\nissue, anyway. In case it's not suggested to revert this back, I'll add\nthe tests.\n\n> We do not use // comment in most parts of our codebase that are\n> supposed to be platform neutral (iow, compat/ is exempt).\n> \nIt seems to be a result of my ignorance that C allowed '//' comments\nonly recently.\n\n> > @@ -877,8 +879,7 @@ static int prepare_to_commit(const char\n> > *index_file, const char *prefix,\n> >  \t\t\t\t(int)(ci.name_end -\n> > ci.name_begin), ci.name_begin,\n> >  \t\t\t\t(int)(ci.mail_end -\n> > ci.mail_begin), ci.mail_begin);\n> >  \n> > -\t\tif (ident_shown)\n> > -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n> > \"%s\", \"\");\n> > +\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n> > // Add new line for clarity\n> \n> This does ensure that an extra blank line appears after the optional\n> section (either after the \"only/include assumed\" message, or \"writing\n> for somebody else\" message).\n> \n> If we were to go with this sparser output, I think we also should\n> give an extra blank line before and after the \"HEAD detached from\n> cafebabe\" message you would see:\n> \n> \t$ git checkout HEAD^0\n> \t$ git commit --allow-empty -o\n> \n> or \"On branch blah\" if you are on a branch.  I think your change\n> adds a blank before, but it does not have a separation before\n> \"Changes not staged for commit\" line.\n> \nI actually didn't think of modifying that in order to keep it in line\nwith the output of `git status`. Further, to me, adding *this* new line\nbefore the \"Changes not staged for commit\" (or something in it's place)\nseems to be wasting some vertical space with an added drawback that\nit's not in line with `git status`.\n\n> Having said that, to be quite honest, I think this \"assuming --only\"\n> message outlive\n> d its usefulness.  This was necessary in very early\n> days of Git because originally \"git commit foo\" did \"git add foo &&\n> git commit\" (i.e. \"-i\" was the default) and then later when we made\n> \"--only\" the new default in order to match everybody else's SCM, we\n> needed to remind users of older versions of Git that \"git commit foo\"\n> now means \"git commit --only foo\", not \"git commit -i foo\" which they\n> may have been used to.  These days, hopefully nobody expects the \"-i\"\n> semantics when they do \"git commit foo\", so perhaps it may be a\n> better\n> change to _remove_ the message altogether.\n> \n> And with that done, I wouldn't have reservations on this change\n> (i.e. \"is it worth wasting extra screen real estate, especially in\n> the vertical direction?\"), as instead of wasting 2 lines to give a\n> message that is no longer useful in today's world, it will be\n> removing one line ;-)\n> \nYou're right! It is worth saving lines if it's not useful these days.\n\nI have sent another mail to the mailing list to get any other possible\nimprovements to the status.\nhttp://public-inbox.org/git/1498583779.2737.4.camel@gmail.com/\n\n-- \nRegards,\nKaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n"},{"id":"323357","messageId":"xmqqwp7x1fie.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"1498584144.2737.7.camel@gmail.com","subject":"Re: [PATCH/RFC] commit-template: improve readability of commit template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-27T17:56:09Z","receivedAt":"2017-06-27T17:56:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n>> I personally do not find these new blank lines are necessary, and\n>> this change wastes vertical screen real estate which is a limited\n>> resource, but that may be just me.  I on the other hand do not think\n>> the result of this patch is overly worse than the status quo, either.\n>> \n> I thought it's not good to trade-off readability for vertical space as\n> the ultimate aim of the commit template (at least to me) is to convey\n> information to the user about the commit that he's going to make. For\n> which, I thought it made more sense to improve it's readability by\n> adding new lines between different sections rather than constrain the\n> output within a few lines.\n\nYou have to be careful when making a trade-off argument.  It depends\non how familiar you already are with the presentation.  Those who\nare/got used to the order of things that come, they will know there\nis extra information when the block of lines are longer than usual\nwithout reading every character and then their eyes are guided to\nread what is extra, without having to waste precious screen real\nestate.  Nobody will _stay_ a new user who is not yet familiar with\nthe everyday output.\n\n>> If we were to go with this sparser output, I think we also should\n>> give an extra blank line before and after the \"HEAD detached from\n>> cafebabe\" message you would see:\n>> \n>> \t$ git checkout HEAD^0\n>> \t$ git commit --allow-empty -o\n>> \n>> or \"On branch blah\" if you are on a branch.  I think your change\n>> adds a blank before, but it does not have a separation before\n>> \"Changes not staged for commit\" line.\n>> \n> I actually didn't think of modifying that in order to keep it in line\n> with the output of `git status`.\n\nI was (and still am) assuming that if we make this change to \"git\ncommit\", we should make matching change to \"git status\" as a given.\n\n> Further, to me, adding *this* new line\n> before the \"Changes not staged for commit\" (or something in it's place)\n> seems to be wasting some vertical space ...\n\nI think it is in line with your original reasoning why you wanted\nthese extra blank lines to separate blocks of different kinds of\ninformation:\n\n - \"Please do this\" instruction at the beginning\n - Make sure you know the default is --only, not --include\n - By the way you are committing for that person, not you\n - This change is being committed on that branch\n - Here are the changes that are already in the index\n - Here are the changes that are not in the index\n - Here are untracked files\n\nLack of a blank between the fourth block and the fifth block [*1*]\nmakes it somewhat inconsistent, doesn't it?\n\n\n[Footnote]\n\n*1* Yes, we should think about removing the optional second block,\n    as I think that it outlived its usefulness; if we are to do so,\n    these become the third and the fourth blocks.\n"},{"id":"323423","messageId":"1498655062.1935.2.camel@gmail.com","threadId":"46263","inReplyTo":"xmqqwp7x1fie.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-28T13:04:22Z","receivedAt":"2017-06-28T13:04:32Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Tue, 2017-06-27 at 10:56 -0700, Junio C Hamano wrote:\n> Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n> > I thought it's not good to trade-off readability for vertical space\n> > as\n> > the ultimate aim of the commit template (at least to me) is to\n> > convey\n> > information to the user about the commit that he's going to make.\n> > For\n> > which, I thought it made more sense to improve it's readability by\n> > adding new lines between different sections rather than constrain\n> > the\n> > output within a few lines.\n> \n> You have to be careful when making a trade-off argument.  It depends\n> on how familiar you already are with the presentation.  Those who\n> are/got used to the order of things that come, they will know there\n> is extra information when the block of lines are longer than usual\n> without reading every character and then their eyes are guided to\n> read what is extra, without having to waste precious screen real\n> estate.  Nobody will _stay_ a new user who is not yet familiar with\n> the everyday output.\n> \nYou're right. I didn't consider the fact that experienced users would\nbe affected as a result of this change, sorry about that. I thought,\nmaking this change would help the new users who would possibly find the\ncommit template to be congested and let experienced users to get\naccustomed to this new output format. I thought this change would be a\nwin-win (at least after people get accustomed to the new formatting). \n\nIn case screen real estate is considered more important here, no\nissues. I'll drop that part of the change, happily.\n\n> > I actually didn't think of modifying that in order to keep it in\n> > line\n> > with the output of `git status`.\n> \n> I was (and still am) assuming that if we make this change to \"git\n> commit\", we should make matching change to \"git status\" as a given.\nI get it now. In that case, I don't think making the change would be a\ngood choice for the following reasons,\n\n    * I think vertical spacing matters more in the output printed to a\n    console.\n    * I myself find it odd to add a new line below the branch\n    information possibly because I'm too accustomed to it's current\n    output.\n\nI tried adding the new line, it seemed to be too spacious. It might be\njust me in this case.\n\n> > Further, to me, adding *this* new line\n> > before the \"Changes not staged for commit\" (or something in it's\n> > place)\n> > seems to be wasting some vertical space ...\n> \n> I think it is in line with your original reasoning why you wanted\n> these extra blank lines to separate blocks of different kinds of\n> information:\n> \n>  - \"Please do this\" instruction at the beginning\n>  - Make sure you know the default is --only, not --include\n>  - By the way you are committing for that person, not you\n>  - This change is being committed on that branch\n>  - Here are the changes that are already in the index\n>  - Here are the changes that are not in the index\n>  - Here are untracked files\n> \n> Lack of a blank between the fourth block and the fifth block [*1*]\n> makes it somewhat inconsistent, doesn't it?\n> \nIt does, for the given set of blocks. I didn't find it inconsistent as\nI thought the separate blocks as follows,\n\n - \"Please do this\" instruction at the beginning\n - Make sure you know the default is --only, not --include\n - By the way you are committing for that person, not you\n - Status of repository (git status)\n\n> [Footnote]\n> \n> *1* Yes, we should think about removing the optional second block,\n>     as I think that it outlived its usefulness; if we are to do so,\n>     these become the third and the fourth blocks.\nIf I interpreted your previous email correctly, I thought we were doing\nit!\n\nI'll send a \"typical\" patch as a follow-up of this mail.\n\n-- \nRegards,\nKaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n"},{"id":"323425","messageId":"20170628132910.7940-1-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":"xmqqwp7x1fie.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-28T13:29:10Z","receivedAt":"2017-06-28T13:29:29Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The commit template adds the optional parts without\na new line to distinguish them. This results in\ndifficulty in interpreting it's content. Add new\nlines to separate the distinct parts of the template.\n\nThe warning about usage of 'explicit paths without\nany corresponding options' has outlived it's purpose of\nwarning users about the usage '--only' as default rather\nthan '--include'. Remove it.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n builtin/commit.c | 9 +--------\n 1 file changed, 1 insertion(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8d1cac062..22d17e6f2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -139,7 +139,6 @@ static enum commit_whence whence;\n static int sequencer_in_use;\n static int use_editor = 1, include_status = 1;\n static int show_ignored_in_status, have_option_m;\n-static const char *only_include_assumed;\n static struct strbuf message = STRBUF_INIT;\n \n static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n@@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n \t\t\t\t  \" yourself if you want to.\\n\"\n \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n-\t\tif (only_include_assumed)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t\t\"%s\", only_include_assumed);\n \n \t\t/*\n \t\t * These should never fail because they come from our own\n@@ -877,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n \n-\t\tif (ident_shown)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n+\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); /* Add new line for clarity */\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n@@ -1208,8 +1203,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n-\tif (argc > 0 && !also && !only)\n-\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n-- \n2.11.0\n\n"},{"id":"323427","messageId":"1498661277.10011.0.camel@gmail.com","threadId":"46263","inReplyTo":"20170628132910.7940-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-28T14:47:57Z","receivedAt":"2017-06-28T14:48:07Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"I forgot to add the RFC in the subject in a hurry. Please consider it's\npresence.\n\n-- \nRegards,\nKaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n"},{"id":"323428","messageId":"1498661450.10011.3.camel@gmail.com","threadId":"46263","inReplyTo":"1498655062.1935.2.camel@gmail.com","subject":"Re: [PATCH/RFC] commit-template: improve readability of commit template","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-28T14:50:50Z","receivedAt":"2017-06-28T14:50:59Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"I might have been ignorant about something about git in my reply in the\nprevious email (found below). In that case, please enlighten me.\n\nOn Wed, 2017-06-28 at 18:34 +0530, Kaartic Sivaraam wrote:\n> On Tue, 2017-06-27 at 10:56 -0700, Junio C Hamano wrote:\n> > Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n> > > I thought it's not good to trade-off readability for vertical\n> > > space\n> > > as\n> > > the ultimate aim of the commit template (at least to me) is to\n> > > convey\n> > > information to the user about the commit that he's going to make.\n> > > For\n> > > which, I thought it made more sense to improve it's readability\n> > > by\n> > > adding new lines between different sections rather than constrain\n> > > the\n> > > output within a few lines.\n> > \n> > You have to be careful when making a trade-off argument.  It\n> > depends\n> > on how familiar you already are with the presentation.  Those who\n> > are/got used to the order of things that come, they will know there\n> > is extra information when the block of lines are longer than usual\n> > without reading every character and then their eyes are guided to\n> > read what is extra, without having to waste precious screen real\n> > estate.  Nobody will _stay_ a new user who is not yet familiar with\n> > the everyday output.\n> > \n> \n> You're right. I didn't consider the fact that experienced users would\n> be affected as a result of this change, sorry about that. I thought,\n> making this change would help the new users who would possibly find\n> the\n> commit template to be congested and let experienced users to get\n> accustomed to this new output format. I thought this change would be\n> a\n> win-win (at least after people get accustomed to the new\n> formatting). \n> \n> In case screen real estate is considered more important here, no\n> issues. I'll drop that part of the change, happily.\n> \n> > > I actually didn't think of modifying that in order to keep it in\n> > > line\n> > > with the output of `git status`.\n> > \n> > I was (and still am) assuming that if we make this change to \"git\n> > commit\", we should make matching change to \"git status\" as a given.\n> \n> I get it now. In that case, I don't think making the change would be\n> a\n> good choice for the following reasons,\n> \n>     * I think vertical spacing matters more in the output printed to\n> a\n>     console.\n>     * I myself find it odd to add a new line below the branch\n>     information possibly because I'm too accustomed to it's current\n>     output.\n> \n> I tried adding the new line, it seemed to be too spacious. It might\n> be\n> just me in this case.\n> \n> > > Further, to me, adding *this* new line\n> > > before the \"Changes not staged for commit\" (or something in it's\n> > > place)\n> > > seems to be wasting some vertical space ...\n> > \n> > I think it is in line with your original reasoning why you wanted\n> > these extra blank lines to separate blocks of different kinds of\n> > information:\n> > \n> >  - \"Please do this\" instruction at the beginning\n> >  - Make sure you know the default is --only, not --include\n> >  - By the way you are committing for that person, not you\n> >  - This change is being committed on that branch\n> >  - Here are the changes that are already in the index\n> >  - Here are the changes that are not in the index\n> >  - Here are untracked files\n> > \n> > Lack of a blank between the fourth block and the fifth block [*1*]\n> > makes it somewhat inconsistent, doesn't it?\n> > \n> \n> It does, for the given set of blocks. I didn't find it inconsistent\n> as\n> I thought the separate blocks as follows,\n> \n>  - \"Please do this\" instruction at the beginning\n>  - Make sure you know the default is --only, not --include\n>  - By the way you are committing for that person, not you\n>  - Status of repository (git status)\n> \n> > [Footnote]\n> > \n> > *1* Yes, we should think about removing the optional second block,\n> >     as I think that it outlived its usefulness; if we are to do so,\n> >     these become the third and the fourth blocks.\n> \n> If I interpreted your previous email correctly, I thought we were\n> doing\n> it!\n> \n> I'll send a \"typical\" patch as a follow-up of this mail.\n> \n\n"},{"id":"323431","messageId":"xmqqshikvz1s.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170628132910.7940-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH] commit-template: improve readability of commit template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-28T16:48:15Z","receivedAt":"2017-06-28T16:48:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> The commit template adds the optional parts without\n> a new line to distinguish them. This results in\n> difficulty in interpreting it's content. Add new\n> lines to separate the distinct parts of the template.\n>\n> The warning about usage of 'explicit paths without\n> any corresponding options' has outlived it's purpose of\n> warning users about the usage '--only' as default rather\n> than '--include'. Remove it.\n>\n> Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n> ---\n>  builtin/commit.c | 9 +--------\n>  1 file changed, 1 insertion(+), 8 deletions(-)\n\nI think I can agree with both of the changes, but this commit does\ntwo things that may be better done in two separate commits.  If I\nwere doing this patch, I probably would make [PATCH 1/2] about\nremoving the only_include_assumed (which is all except for the hunk\nat -877,8) and then [PATCH 2/2] about giving a separating blank line\nbefore the \"git status\" output.\n\n\n> The warning about usage of 'explicit paths without\n> any corresponding options' has outlived it's purpose of\n> warning users about the usage '--only' as default rather\n> than '--include'. Remove it.\n\nWith a bit more digging of the history:\n\n    The notice that \"git commit <paths>\" default to \"git commit\n    --only <paths>\" was there since 756e3ee0 (\"Merge branch\n    'jc/commit'\", 2006-02-14).  Back then, existing users of Git\n    expected the command doing \"git commit --include <paths>\", and\n    after we changed the behaviour of the command to align with\n    other people's \"$scm commit <paths>\", we added the text to help\n    them transition their expectations.  Remove the message that now\n    has outlived its usefulness.\n\nperhaps.\n\nThanks.\n\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 8d1cac062..22d17e6f2 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -139,7 +139,6 @@ static enum commit_whence whence;\n>  static int sequencer_in_use;\n>  static int use_editor = 1, include_status = 1;\n>  static int show_ignored_in_status, have_option_m;\n> -static const char *only_include_assumed;\n>  static struct strbuf message = STRBUF_INIT;\n>  \n>  static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n> @@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n>  \t\t\t\t  \" yourself if you want to.\\n\"\n>  \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n> -\t\tif (only_include_assumed)\n> -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n> -\t\t\t\t\t\"%s\", only_include_assumed);\n>  \n>  \t\t/*\n>  \t\t * These should never fail because they come from our own\n> @@ -877,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n>  \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n>  \n> -\t\tif (ident_shown)\n> -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n> +\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); /* Add new line for clarity */\n>  \n>  \t\tsaved_color_setting = s->use_color;\n>  \t\ts->use_color = 0;\n> @@ -1208,8 +1203,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>  \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n>  \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n>  \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n> -\tif (argc > 0 && !also && !only)\n> -\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n>  \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n>  \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n>  \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n"},{"id":"323477","messageId":"20170629170120.11479-1-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":"xmqqshikvz1s.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-29T17:01:19Z","receivedAt":"2017-06-29T17:02:05Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The notice that \"git commit <paths>\" default to \"git commit\n--only <paths>\" was there since 756e3ee0 (\"Merge branch\n'jc/commit'\", 2006-02-14).  Back then, existing users of Git\nexpected the command doing \"git commit --include <paths>\", and\nafter we changed the behaviour of the command to align with\nother people's \"$scm commit <paths>\", we added the text to help\nthem transition their expectations.  Remove the message that now\nhas outlived its usefulness.\n---\n builtin/commit.c | 6 ------\n 1 file changed, 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8d1cac062..64701c8f4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -139,7 +139,6 @@ static enum commit_whence whence;\n static int sequencer_in_use;\n static int use_editor = 1, include_status = 1;\n static int show_ignored_in_status, have_option_m;\n-static const char *only_include_assumed;\n static struct strbuf message = STRBUF_INIT;\n \n static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n@@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n \t\t\t\t  \" yourself if you want to.\\n\"\n \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n-\t\tif (only_include_assumed)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t\t\"%s\", only_include_assumed);\n \n \t\t/*\n \t\t * These should never fail because they come from our own\n@@ -1208,8 +1204,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n-\tif (argc > 0 && !also && !only)\n-\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n-- \n2.11.0\n\n"},{"id":"323478","messageId":"20170629170120.11479-2-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":"20170629170120.11479-1-kaarticsivaraam91196@gmail.com","subject":"[PATCH 2/2] commit-template: add new line before status information","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-29T17:01:20Z","receivedAt":"2017-06-29T17:02:08Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The commit template adds the optional parts without\na new line to distinguish them. This results in\ndifficulty in interpreting it's content, specifically\nfor inexperienced users.\n\nAdd new lines to separate the distinct parts of the\ntemplate.\n---\n I tried writing tests to ensure that the new line is added\n but as it seems to require checking multi-line, special \n options of grep were required to check. I tried the following,\n\n   test_expect_success 'new line found before status message' '\n    ! (GIT_EDITOR=\"cat >editor-input\" git commit) &&\n    grep -Pz \"#\\n# On branch\" editor-input\n   '\n\n It worked well locally but seems to make the build with \n GETTEXT_POISON=YesPlease to fail. So, I removed it.\n Not sure how to write a good test for this change, sorry :(\n\n builtin/commit.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 64701c8f4..22d17e6f2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -873,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n \n-\t\tif (ident_shown)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n+\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); /* Add new line for clarity */\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n-- \n2.11.0\n\n"},{"id":"323483","messageId":"xmqq4luyvg02.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170629170120.11479-2-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH 2/2] commit-template: add new line before status information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-29T17:51:57Z","receivedAt":"2017-06-29T17:52:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> The commit template adds the optional parts without\n> a new line to distinguish them. This results in\n> difficulty in interpreting it's content, specifically\n> for inexperienced users.\n>\n> Add new lines to separate the distinct parts of the\n> template.\n> ---\n>  I tried writing tests to ensure that the new line is added\n>  but as it seems to require checking multi-line, special \n>  options of grep were required to check. I tried the following,\n>\n>    test_expect_success 'new line found before status message' '\n>     ! (GIT_EDITOR=\"cat >editor-input\" git commit) &&\n>     grep -Pz \"#\\n# On branch\" editor-input\n>    '\n>\n>  It worked well locally but seems to make the build with \n>  GETTEXT_POISON=YesPlease to fail. So, I removed it.\n>  Not sure how to write a good test for this change, sorry :(\n\nThe above is a good way to capture the input to the editor, but the\ntest with \"grep -P\" which is not portable would not work well.  You\nhowever should be able to prepare an expected output with\n\n\tcat >expect <<\\-EOF &&\n\t... expected contents to editor-input here ...\n\tEOF\n\nand do \"test_i18ncmp expect editor-input\" instead of \"grep -P\".\n"},{"id":"323484","messageId":"xmqqzicqu17z.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170629170120.11479-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-29T17:56:32Z","receivedAt":"2017-06-29T17:56:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> The notice that \"git commit <paths>\" default to \"git commit\n> --only <paths>\" was there since 756e3ee0 (\"Merge branch\n> 'jc/commit'\", 2006-02-14).  Back then, existing users of Git\n> expected the command doing \"git commit --include <paths>\", and\n> after we changed the behaviour of the command to align with\n> other people's \"$scm commit <paths>\", we added the text to help\n> them transition their expectations.  Remove the message that now\n> has outlived its usefulness.\n> ---\n>  builtin/commit.c | 6 ------\n>  1 file changed, 6 deletions(-)\n\nWhen I said \"I would have ... if I were doing this\", I merely meant\nexactly that---as I weren't doing it, I left it up to you.  But you\ndid it the way anyways, which is very nice of you ;-).\n\nLooks good.  Should we consider this signed-off by you?\n\nThanks.\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 8d1cac062..64701c8f4 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -139,7 +139,6 @@ static enum commit_whence whence;\n>  static int sequencer_in_use;\n>  static int use_editor = 1, include_status = 1;\n>  static int show_ignored_in_status, have_option_m;\n> -static const char *only_include_assumed;\n>  static struct strbuf message = STRBUF_INIT;\n>  \n>  static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n> @@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n>  \t\t\t\t  \" yourself if you want to.\\n\"\n>  \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n> -\t\tif (only_include_assumed)\n> -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n> -\t\t\t\t\t\"%s\", only_include_assumed);\n>  \n>  \t\t/*\n>  \t\t * These should never fail because they come from our own\n> @@ -1208,8 +1204,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>  \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n>  \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n>  \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n> -\tif (argc > 0 && !also && !only)\n> -\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n>  \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n>  \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n>  \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n"},{"id":"323488","messageId":"xmqqmv8qu08n.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170629170120.11479-2-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH 2/2] commit-template: add new line before status information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-29T18:17:44Z","receivedAt":"2017-06-29T18:17:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> The commit template adds the optional parts without\n> a new line to distinguish them. This results in\n> difficulty in interpreting it's content, specifically\n> for inexperienced users.\n>\n> Add new lines to separate the distinct parts of the\n> template.\n\nThe rationale of this has changed in this final version, hasn't it,\nespecially with the removal of the \"include/only warning\" bit?\n\nWe used to add a blank line to separate the \"we are committing for\nsomebody else\", which is an optional part, from the status output,\nonly when we have the optional part.  Now with your change, we\nunconditionally have a blank before the part that would have been\nshown by \"git status\" to separate the two parts out.\n\n\n\n> ---\n>  I tried writing tests to ensure that the new line is added\n>  but as it seems to require checking multi-line, special \n>  options of grep were required to check. I tried the following,\n>\n>    test_expect_success 'new line found before status message' '\n>     ! (GIT_EDITOR=\"cat >editor-input\" git commit) &&\n>     grep -Pz \"#\\n# On branch\" editor-input\n>    '\n>\n>  It worked well locally but seems to make the build with \n>  GETTEXT_POISON=YesPlease to fail. So, I removed it.\n>  Not sure how to write a good test for this change, sorry :(\n>\n>  builtin/commit.c | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 64701c8f4..22d17e6f2 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -873,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n>  \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n>  \n> -\t\tif (ident_shown)\n> -\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n> +\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); /* Add new line for clarity */\n>  \n>  \t\tsaved_color_setting = s->use_color;\n>  \t\ts->use_color = 0;\n"},{"id":"323544","messageId":"1498792731.2063.5.camel@gmail.com","threadId":"46263","inReplyTo":"xmqqzicqu17z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-30T03:18:51Z","receivedAt":"2017-06-30T03:19:01Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-06-29 at 10:56 -0700, Junio C Hamano wrote:\n> When I said \"I would have ... if I were doing this\", I merely meant\n> exactly that---as I weren't doing it, I left it up to you.  But you\n> did it the way anyways, which is very nice of you ;-).\n> \nIt made a *lot* of sense, that's why. :)\n\n> Looks good.  Should we consider this signed-off by you?\n> \nYeah, forgot that. Will follow-up with a patch.\n"},{"id":"323545","messageId":"1498792789.2063.6.camel@gmail.com","threadId":"46263","inReplyTo":"xmqqmv8qu08n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] commit-template: add new line before status information","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-30T03:19:49Z","receivedAt":"2017-06-30T03:19:56Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-06-29 at 11:17 -0700, Junio C Hamano wrote:\n> The rationale of this has changed in this final version, hasn't it,\n> especially with the removal of the \"include/only warning\" bit?\n> \n> We used to add a blank line to separate the \"we are committing for\n> somebody else\", which is an optional part, from the status output,\n> only when we have the optional part.  Now with your change, we\n> unconditionally have a blank before the part that would have been\n> shown by \"git status\" to separate the two parts out.\n> \nYes, modified the commit message accordingly hoping that's what you\nmeant.\n"},{"id":"323564","messageId":"20170630121221.3327-1-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":"1498792731.2063.5.camel@gmail.com","subject":"[PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-30T12:12:20Z","receivedAt":"2017-06-30T12:14:32Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The notice that \"git commit <paths>\" default to \"git commit\n--only <paths>\" was there since 756e3ee0 (\"Merge branch\n'jc/commit'\", 2006-02-14).  Back then, existing users of Git\nexpected the command doing \"git commit --include <paths>\", and\nafter the behaviour of the command was changed to align with\nother people's \"$scm commit <paths>\", the text was added to help\nthem transition their expectations.\n\nRemove the message that now has outlived its usefulness.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n Only commit message has been changed. Modified it a little\n to remove the self referencing words that didn't seem to suit\n well with the commit's author (that's just me :))\n \n builtin/commit.c | 6 ------\n 1 file changed, 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8d1cac062..64701c8f4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -139,7 +139,6 @@ static enum commit_whence whence;\n static int sequencer_in_use;\n static int use_editor = 1, include_status = 1;\n static int show_ignored_in_status, have_option_m;\n-static const char *only_include_assumed;\n static struct strbuf message = STRBUF_INIT;\n \n static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n@@ -841,9 +840,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t  \"with '%c' will be kept; you may remove them\"\n \t\t\t\t  \" yourself if you want to.\\n\"\n \t\t\t\t  \"An empty message aborts the commit.\\n\"), comment_line_char);\n-\t\tif (only_include_assumed)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t\t\"%s\", only_include_assumed);\n \n \t\t/*\n \t\t * These should never fail because they come from our own\n@@ -1208,8 +1204,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n-\tif (argc > 0 && !also && !only)\n-\t\tonly_include_assumed = _(\"Explicit paths specified without -i or -o; assuming --only paths...\");\n \tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n \t\tcleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;\n \telse if (!strcmp(cleanup_arg, \"verbatim\"))\n-- \n2.11.0\n\n"},{"id":"323565","messageId":"20170630121221.3327-2-kaarticsivaraam91196@gmail.com","threadId":"46263","inReplyTo":"20170630121221.3327-1-kaarticsivaraam91196@gmail.com","subject":"[PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-06-30T12:12:21Z","receivedAt":"2017-06-30T12:14:42Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The commit template adds the status information without\nadding a new line to distinguish them in the absence\nof optional parts. This results in difficulty in interpreting\nit's content, specifically for inexperienced users.\n\nUnconditionally, add new lines to separate the status message\nfrom the other parts of the commit-template to make it more\nreadable.\n\nSigned-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n---\n In trying to make the test independent of the previous test,\n it got a bit long. Hope it's not doing anything that shouldn't\n be done.\n\n In case you would wonder, the 'git reset' line is to remove \n the files that were staged in previous test. I've done it in\n the same motive that lead to a bigger test.\n\n travis-ci builds passed\n\n builtin/commit.c  |  3 +--\n t/t7500-commit.sh | 23 +++++++++++++++++++++++\n 2 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 64701c8f4..22d17e6f2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -873,8 +873,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t(int)(ci.name_end - ci.name_begin), ci.name_begin,\n \t\t\t\t(int)(ci.mail_end - ci.mail_begin), ci.mail_begin);\n \n-\t\tif (ident_shown)\n-\t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\");\n+\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"%s\", \"\"); /* Add new line for clarity */\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 116885a26..5739d3ed2 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -329,4 +329,27 @@ test_expect_success 'invalid message options when using --fixup' '\n \ttest_must_fail git commit --fixup HEAD~1 -F log\n '\n \n+cat >expected-template <<EOF\n+\n+# Please enter the commit message for your changes. Lines starting\n+# with '#' will be ignored, and an empty message aborts the commit.\n+#\n+# Author:    A U Thor <author@example.com>\n+#\n+# On branch commit-template-check\n+# Changes to be committed:\n+#\tnew file:   commit-template-check\n+#\n+# Untracked files not listed\n+EOF\n+\n+test_expect_success 'new line found before status message in commit template' '\n+\tgit checkout -b commit-template-check &&\n+\tgit reset --hard HEAD &&\n+\ttouch commit-template-check &&\n+\tgit add commit-template-check &&\n+\tGIT_EDITOR=\"cat >editor-input\" git commit --untracked-files=no --allow-empty-message &&\n+\ttest_i18ncmp expected-template editor-input\n+'\n+\n test_done\n-- \n2.11.0\n\n"},{"id":"323567","messageId":"xmqqa84pttmh.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"20170630121221.3327-2-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-30T14:52:54Z","receivedAt":"2017-06-30T14:53:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, both looks good.  Will queue.\n"},{"id":"323667","messageId":"1498874344.2245.0.camel@gmail.com","threadId":"46263","inReplyTo":"xmqqa84pttmh.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-01T01:59:04Z","receivedAt":"2017-07-01T01:59:11Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Fri, 2017-06-30 at 07:52 -0700, Junio C Hamano wrote:\n> Thanks, both looks good.  Will queue.\nYou're welcome :)\n"},{"id":"323670","messageId":"87mv8oe5zc.fsf@gmail.com","threadId":"46263","inReplyTo":"1498874344.2245.0.camel@gmail.com","subject":"Re: [PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-01T11:44:55Z","receivedAt":"2017-07-01T11:45:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Jul 01 2017, Kaartic Sivaraam jotted:\n\n> On Fri, 2017-06-30 at 07:52 -0700, Junio C Hamano wrote:\n>> Thanks, both looks good.Will queue.\n> You're welcome :)\n\nJust as someone reading this from the sidelines, very nice to have\nsomeone working this part of the UI, but it would be much easier to\nreview if you included before/after examples of changes, e.g. (for this\nhypothetical change):\n\n    \n    Before we'd say:\n    \n        # Please enter the commit message for your changes. Lines starting\n        # with '#' will be ignored, and an empty message aborts the commit.\n        #\n        # Date:      <date>\n        #\n        # On branch master\n        # Your branch is up-to-date with 'origin/master'.\n    \n    Now:\n    \n        # Please enter the commit message for your changes. Lines starting\n        # with '#' will be ignored, and an empty message aborts the commit.\n        #\n        # Date:      <date>\n        #\n        # On branch master\n        # Your current branch is up-to-date with 'origin/master'.\n    \n    And as a word-diff:\n    \n        [...]\n        # Your {+current+} branch is up-to-date with 'origin/master'.\n\nOr something like that, much easier to read something like that than\nread the code and mentally glue together what it's going to change.\n"},{"id":"323671","messageId":"1498910895.1849.1.camel@gmail.com","threadId":"46263","inReplyTo":"87mv8oe5zc.fsf@gmail.com","subject":"Re: [PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-01T12:08:15Z","receivedAt":"2017-07-01T12:08:26Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Sat, 2017-07-01 at 13:44 +0200, Ævar Arnfjörð Bjarmason wrote:\n> Just as someone reading this from the sidelines, very nice to have\n> someone working this part of the UI, but it would be much easier to\n> review if you included before/after examples of changes\n\nThat's a good comment. Will try to follow that in future. :)\n\n-- \nRegards,\nKaartic Sivaraam <kaarticsivaraam91196@gmail.com>\n"},{"id":"323685","messageId":"xmqqvancnkd9.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"87mv8oe5zc.fsf@gmail.com","subject":"Re: [PATCH 2/2] commit-template: distinguish status information unconditionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-01T17:21:38Z","receivedAt":"2017-07-01T17:21:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Just as someone reading this from the sidelines, very nice to have\n> someone working this part of the UI, but it would be much easier to\n> review if you included before/after examples of changes, e.g. (for this\n> hypothetical change):\n> \n>     \n>     Before we'd say:\n>     \n>         # Please enter the commit message for your changes. Lines starting\n>         # with '#' will be ignored, and an empty message aborts the commit.\n>         #\n>         # Date:      <date>\n>         #\n>         # On branch master\n>         # Your branch is up-to-date with 'origin/master'.\n>     \n>     Now:\n>     \n>         # Please enter the commit message for your changes. Lines starting\n>         # with '#' will be ignored, and an empty message aborts the commit.\n>         #\n>         # Date:      <date>\n>         #\n>         # On branch master\n>         # Your current branch is up-to-date with 'origin/master'.\n>     \n>     And as a word-diff:\n>     \n>         [...]\n>         # Your {+current+} branch is up-to-date with 'origin/master'.\n>\n> Or something like that, much easier to read something like that than\n> read the code and mentally glue together what it's going to change.\n\nI think you gave an example that is different from what was done on\npurpose, so that Kaartic can respond with \"I see what you mean; what\nI did is different from your example but this.\", but it seems that\nthe attempt failed X-<.\n\nI do not think the patch changes the output in a situation where the\nabove \"Before\" would be shown.  Instead, when the extra \"Date: <date>\"\n(or \"Author: <author>\") is not shown, the Before picture would look\nlike:\n\n         # Please enter the commit message for your changes. Lines starting\n         # with '#' will be ignored, and an empty message aborts the commit.\n         # On branch master\n\nand the update makes it look like:\n\n         # Please enter the commit message for your changes. Lines starting\n         # with '#' will be ignored, and an empty message aborts the commit.\n         #\n         # On branch master\n\nHope that helps.\n"},{"id":"324037","messageId":"1499622873.8552.2.camel@gmail.com","threadId":"46263","inReplyTo":"20170630121221.3327-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-09T17:54:33Z","receivedAt":"2017-07-09T17:55:42Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Fri, 2017-06-30 at 17:42 +0530, Kaartic Sivaraam wrote:\n> The notice that \"git commit <paths>\" default to \"git commit\n> --only <paths>\" was there since 756e3ee0 (\"Merge branch\n> 'jc/commit'\", 2006-02-14).  Back then, existing users of Git\n> expected the command doing \"git commit --include <paths>\", and\n> after the behaviour of the command was changed to align with\n> other people's \"$scm commit <paths>\", the text was added to help\n> them transition their expectations.\n> \n> Remove the message that now has outlived its usefulness.\n> \nIt just recently dawned on me that the message,\n\n    Explicit paths specified without -i or -o; assuming --only paths..\n\nis translated and \"git grep\" shows it's presence in the files present\nin the 'po' directory. What should be done to them? Should the\ntranslators be notified? \n\nBTW what does the word 'po' stand for in the first place? I digged a\nbit but couldn't find much from the log.\n\n-- \nKaartic\n"},{"id":"324040","messageId":"xmqqh8yl8mpq.fsf@gitster.mtv.corp.google.com","threadId":"46263","inReplyTo":"1499622873.8552.2.camel@gmail.com","subject":"Re: [PATCH 1/2] commit-template: remove outdated notice about explicit paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-09T18:54:57Z","receivedAt":"2017-07-09T18:55:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> writes:\n\n> It just recently dawned on me that the message,\n>\n>     Explicit paths specified without -i or -o; assuming --only paths..\n>\n> is translated and \"git grep\" shows it's presence in the files present\n> in the 'po' directory. What should be done to them?\n\nLeave them as they are.  They are catalogued by the messages in the\ncode and gives us the translated equivalent.  If you removed a message,\nthe only effect is that the corresponding message will not be asked\nfor translation.\n\nNear the end of a development cycle, the internationalization\ncoordinator will update the master message catalog and he'll remove\nit when notices that the message no longer is used (even if he fails\nto spot the obsolete and unused message, no harm is done).\n\nThanks.\n\n\n"}]}