{"thread":{"id":"13332","subject":"[PATCH 0/3] Show author and/or committer in some cases","startedAt":"2008-04-30T08:47:13Z","lastAt":"2008-05-02T09:25:38Z","messageCount":15,"participants":["Santi Béjar","Jeff King","Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"75655","messageId":"1209545236-4266-1-git-send-email-sbejar@gmail.com","threadId":"13332","inReplyTo":null,"subject":"[PATCH 0/3] Show author and/or committer in some cases","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-04-30T08:47:13Z","receivedAt":"2008-04-30T08:47:13Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"Hi *,\n\n  This patch series tries to make more evident the author and committer ident\nused in a commit trying to be minimize when it is shown.\n\nSanti\n\nSanti Béjar (3):\n  Preparation to call determine_author_info from prepare_to_commit\n  commit: Show author if different from committer\n  commit: Show the committer ident when is different from the parent\n\n builtin-commit.c  |  127 ++++++++++++++++++++++++++++++++++++-----------------\n t/t7502-commit.sh |   22 +++++++++\n 2 files changed, 109 insertions(+), 40 deletions(-)\n"},{"id":"75656","messageId":"1209545236-4266-2-git-send-email-sbejar@gmail.com","threadId":"13332","inReplyTo":"1209545236-4266-1-git-send-email-sbejar@gmail.com","subject":"[PATCH 1/3] Preparation to call determine_author_info from prepare_to_commit","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-04-30T08:47:14Z","receivedAt":"2008-04-30T08:47:14Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"Reorder functions definitions such that determine_author_info is\ndefined before prepare_to_commit. No code changes.\n\nSigned-off-by: Santi Béjar <sbejar@gmail.com>\n---\n builtin-commit.c |   78 +++++++++++++++++++++++++++---------------------------\n 1 files changed, 39 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 256181a..a37d8c3 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -395,6 +395,45 @@ static int is_a_merge(const unsigned char *sha1)\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n \n+static void determine_author_info(struct strbuf *sb)\n+{\n+\tchar *name, *email, *date;\n+\n+\tname = getenv(\"GIT_AUTHOR_NAME\");\n+\temail = getenv(\"GIT_AUTHOR_EMAIL\");\n+\tdate = getenv(\"GIT_AUTHOR_DATE\");\n+\n+\tif (use_message) {\n+\t\tconst char *a, *lb, *rb, *eol;\n+\n+\t\ta = strstr(use_message_buffer, \"\\nauthor \");\n+\t\tif (!a)\n+\t\t\tdie(\"invalid commit: %s\", use_message);\n+\n+\t\tlb = strstr(a + 8, \" <\");\n+\t\trb = strstr(a + 8, \"> \");\n+\t\teol = strchr(a + 8, '\\n');\n+\t\tif (!lb || !rb || !eol)\n+\t\t\tdie(\"invalid commit: %s\", use_message);\n+\n+\t\tname = xstrndup(a + 8, lb - (a + 8));\n+\t\temail = xstrndup(lb + 2, rb - (lb + 2));\n+\t\tdate = xstrndup(rb + 2, eol - (rb + 2));\n+\t}\n+\n+\tif (force_author) {\n+\t\tconst char *lb = strstr(force_author, \" <\");\n+\t\tconst char *rb = strchr(force_author, '>');\n+\n+\t\tif (!lb || !rb)\n+\t\t\tdie(\"malformed --author parameter\");\n+\t\tname = xstrndup(force_author, lb - force_author);\n+\t\temail = xstrndup(lb + 2, rb - (lb + 2));\n+\t}\n+\n+\tstrbuf_addf(sb, \"author %s\\n\", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix)\n {\n \tstruct stat statbuf;\n@@ -622,45 +661,6 @@ static int message_is_empty(struct strbuf *sb, int start)\n \treturn 1;\n }\n \n-static void determine_author_info(struct strbuf *sb)\n-{\n-\tchar *name, *email, *date;\n-\n-\tname = getenv(\"GIT_AUTHOR_NAME\");\n-\temail = getenv(\"GIT_AUTHOR_EMAIL\");\n-\tdate = getenv(\"GIT_AUTHOR_DATE\");\n-\n-\tif (use_message) {\n-\t\tconst char *a, *lb, *rb, *eol;\n-\n-\t\ta = strstr(use_message_buffer, \"\\nauthor \");\n-\t\tif (!a)\n-\t\t\tdie(\"invalid commit: %s\", use_message);\n-\n-\t\tlb = strstr(a + 8, \" <\");\n-\t\trb = strstr(a + 8, \"> \");\n-\t\teol = strchr(a + 8, '\\n');\n-\t\tif (!lb || !rb || !eol)\n-\t\t\tdie(\"invalid commit: %s\", use_message);\n-\n-\t\tname = xstrndup(a + 8, lb - (a + 8));\n-\t\temail = xstrndup(lb + 2, rb - (lb + 2));\n-\t\tdate = xstrndup(rb + 2, eol - (rb + 2));\n-\t}\n-\n-\tif (force_author) {\n-\t\tconst char *lb = strstr(force_author, \" <\");\n-\t\tconst char *rb = strchr(force_author, '>');\n-\n-\t\tif (!lb || !rb)\n-\t\t\tdie(\"malformed --author parameter\");\n-\t\tname = xstrndup(force_author, lb - force_author);\n-\t\temail = xstrndup(lb + 2, rb - (lb + 2));\n-\t}\n-\n-\tstrbuf_addf(sb, \"author %s\\n\", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));\n-}\n-\n static int parse_and_validate_options(int argc, const char *argv[],\n \t\t\t\t      const char * const usage[])\n {\n-- \n1.5.5.1.148.g37726\n"},{"id":"75657","messageId":"1209545236-4266-3-git-send-email-sbejar@gmail.com","threadId":"13332","inReplyTo":"1209545236-4266-1-git-send-email-sbejar@gmail.com","subject":"[PATCH 2/3] commit: Show author if different from committer","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-04-30T08:47:15Z","receivedAt":"2008-04-30T08:47:15Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"That would help reassure anybody while committing other's changes.\n\nSigned-off-by: Santi Béjar <sbejar@gmail.com>\n---\n builtin-commit.c  |   23 ++++++++++++++++++++---\n t/t7502-commit.sh |   10 ++++++++++\n 2 files changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex a37d8c3..c51c70f 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,6 +47,7 @@ static enum {\n \n static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n+static char *author_name, *author_email, *author_date;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n /*\n@@ -395,7 +396,7 @@ static int is_a_merge(const unsigned char *sha1)\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n \n-static void determine_author_info(struct strbuf *sb)\n+static void determine_author_info()\n {\n \tchar *name, *email, *date;\n \n@@ -431,7 +432,9 @@ static void determine_author_info(struct strbuf *sb)\n \t\temail = xstrndup(lb + 2, rb - (lb + 2));\n \t}\n \n-\tstrbuf_addf(sb, \"author %s\\n\", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));\n+\tauthor_name = name;\n+\tauthor_email = email;\n+\tauthor_date = date;\n }\n \n static int prepare_to_commit(const char *index_file, const char *prefix)\n@@ -443,6 +446,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \tFILE *fp;\n \tconst char *hook_arg1 = NULL;\n \tconst char *hook_arg2 = NULL;\n+\tconst char *author_ident;\n+\tconst char *committer_ident;\n \n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n@@ -522,6 +527,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \n \tstrbuf_release(&sb);\n \n+\tdetermine_author_info();\n+\n \tif (use_editor) {\n \t\tif (in_merge)\n \t\t\tfprintf(fp,\n@@ -545,6 +552,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\tif (only_include_assumed)\n \t\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n \n+\t\tauthor_ident = xstrdup(fmt_name(author_name, author_email));\n+\t\tcommitter_ident =\n+\t\t\txstrdup(fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\t\t getenv(\"GIT_COMMITTER_EMAIL\")));\n+\t\tif (strcmp(author_ident, committer_ident))\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"# Author:    %s\\n\",\n+\t\t\t\tfmt_name(author_name, author_email));\n+\n \t\tsaved_color_setting = wt_status_use_color;\n \t\twt_status_use_color = 0;\n \t\tcommitable = run_status(fp, index_file, prefix, 1);\n@@ -920,7 +936,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_addf(&sb, \"parent %s\\n\", sha1_to_hex(head_sha1));\n \t}\n \n-\tdetermine_author_info(&sb);\n+\tstrbuf_addf(&sb, \"author %s\\n\",\n+\t\t    fmt_ident(author_name, author_email, author_date, IDENT_ERROR_ON_NO_NAME));\n \tstrbuf_addf(&sb, \"committer %s\\n\", git_committer_info(IDENT_ERROR_ON_NO_NAME));\n \tif (!is_encoding_utf8(git_commit_encoding))\n \t\tstrbuf_addf(&sb, \"encoding %s\\n\", git_commit_encoding);\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 284c941..95acf4c 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -154,6 +154,16 @@ test_expect_success 'cleanup commit messages (strip,-F,-e)' '\n \n '\n \n+echo \"# Author:    $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\" >> expect\n+\n+test_expect_success 'author different from committer' '\n+\n+\techo >>negative &&\n+\tgit commit -e -m \"sample\"\n+\thead -n 5 .git/COMMIT_EDITMSG >actual &&\n+\ttest_cmp expect actual\n+'\n+\n pwd=`pwd`\n cat >> .git/FAKE_EDITOR << EOF\n #! /bin/sh\n-- \n1.5.5.1.148.g37726\n"},{"id":"75658","messageId":"1209545236-4266-4-git-send-email-sbejar@gmail.com","threadId":"13332","inReplyTo":"1209545236-4266-1-git-send-email-sbejar@gmail.com","subject":"[PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-04-30T08:47:16Z","receivedAt":"2008-04-30T08:47:16Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"To remind the committer ident in case it is not what you want\n(taken from the gecos field, want to use different ident in\ndifferent repositories).\n\nSigned-off-by: Santi Béjar <sbejar@gmail.com>\n---\nHi,\n\n  it does not work with the initial commit, I don't know why.\n  Even with this:\n\tif (!commit)\n\t\treturn NULL;\n\n  Can someone help me? Thanks\n\n builtin-commit.c  |   30 ++++++++++++++++++++++++++++++\n t/t7502-commit.sh |   12 ++++++++++++\n 2 files changed, 42 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex c51c70f..06582ef 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -437,6 +437,29 @@ static void determine_author_info()\n \tauthor_date = date;\n }\n \n+const char *get_parent_ident()\n+{\n+\tunsigned char sha1[20];\n+\tstruct commit *commit;\n+\tconst char *a, *lb, *rb, *eol;\n+\n+\n+\tget_sha1(\"HEAD\", sha1);\n+\tcommit = lookup_commit_reference(sha1);\n+\tif (!commit)\n+\t\treturn NULL;\n+\n+\ta = strstr(commit->buffer, \"\\ncommitter \");\n+\n+\tlb = strstr(a + 11, \" <\");\n+\trb = strstr(a + 11, \"> \");\n+\teol = strchr(a + 11, '\\n');\n+\tif (!lb || !rb || !eol)\n+\t\treturn NULL;\n+\n+\treturn xstrndup(a + 11, rb + 1 - (a + 11));\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix)\n {\n \tstruct stat statbuf;\n@@ -448,6 +471,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \tconst char *hook_arg2 = NULL;\n \tconst char *author_ident;\n \tconst char *committer_ident;\n+\tconst char *parent_ident;\n \n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n@@ -561,6 +585,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\t\t\t\"# Author:    %s\\n\",\n \t\t\t\tfmt_name(author_name, author_email));\n \n+\t\tparent_ident = xstrdup(get_parent_ident());\n+\t\tif (strcmp(parent_ident, committer_ident))\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"# Committer: %s\\n\",\n+\t\t\t\tcommitter_ident);\n+\n \t\tsaved_color_setting = wt_status_use_color;\n \t\twt_status_use_color = 0;\n \t\tcommitable = run_status(fp, index_file, prefix, 1);\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 95acf4c..96b5dcb 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -164,6 +164,18 @@ test_expect_success 'author different from committer' '\n \ttest_cmp expect actual\n '\n \n+GIT_COMMITTER_NAME=\"C. O. Mitter\"\n+export GIT_COMMITTER_NAME\n+echo \"# Committer: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >> expect\n+\n+test_expect_success 'committer different from the parent committer' '\n+\n+\techo >>negative &&\n+\tgit commit -e -m \"sample\"\n+\thead -n 6 .git/COMMIT_EDITMSG >actual &&\n+\ttest_cmp expect actual\n+'\n+\n pwd=`pwd`\n cat >> .git/FAKE_EDITOR << EOF\n #! /bin/sh\n-- \n1.5.5.1.148.g37726\n"},{"id":"75681","messageId":"20080430145017.GA922@sigill.intra.peff.net","threadId":"13332","inReplyTo":"1209545236-4266-4-git-send-email-sbejar@gmail.com","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T14:50:17Z","receivedAt":"2008-04-30T14:50:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2008 at 10:47:16AM +0200, Santi Béjar wrote:\n\n>   it does not work with the initial commit, I don't know why.\n>   Even with this:\n> \tif (!commit)\n> \t\treturn NULL;\n> \n>   Can someone help me? Thanks\n\nIt segfaults for me, since you try to strdup NULL. I think you want to\nreturn \"\" if there is no initial commit, so you can strcmp against the\ncurrent committer (or alternatively, explicitly check for NULL).\n\n> +\t\tparent_ident = xstrdup(get_parent_ident());\n> +\t\tif (strcmp(parent_ident, committer_ident))\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"# Committer: %s\\n\",\n> +\t\t\t\tcommitter_ident);\n> +\n\nWhy strdup at all here, which leaks? Nobody else uses this variable, so\nit should be sufficient to strcmp(get_parent_ident(), committer_ident)\nonce get_parent_ident promises to always return a valid string. Or\nalternatively:\n\n  parent_ident = get_parent_ident();\n  if (!parent_ident || strcmp(parent_ident, committer_ident))\n\n\nA few other comments (I like the idea overall):\n\n  - I haven't looked at the code organization, so maybe it is not\n    feasible, but it seems like this stuff should go into wt-status's\n    implementation, which would show up in a git-status.\n\n  - The output looks very cluttered. It would be nice to have\n\n    # enter your commit message...\n    #\n    # Author: whatever\n    # Committer: whatever\n    #\n    # other stuff\n\n    IOW, put in a blank line on either side, but not between the two\n    identities if both are shown.\n\n-Peff\n"},{"id":"75682","messageId":"20080430151047.GA3482@sigill.intra.peff.net","threadId":"13332","inReplyTo":"20080430145017.GA922@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T15:10:47Z","receivedAt":"2008-04-30T15:10:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2008 at 10:50:17AM -0400, Jeff King wrote:\n\n> It segfaults for me, since you try to strdup NULL. I think you want to\n> return \"\" if there is no initial commit, so you can strcmp against the\n> current committer (or alternatively, explicitly check for NULL).\n> [...]\n>     IOW, put in a blank line on either side, but not between the two\n>     identities if both are shown.\n\nFixes for these are below (no commit message; I think you should respin\nthe series rather than making this a separate patch). I didn't look at\nputting it into wt-status.c, but I think that is worth pursuing.\n\n---\n builtin-commit.c |   14 ++++++++++----\n 1 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 30828c5..4e62510 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -472,6 +472,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \tconst char *author_ident;\n \tconst char *committer_ident;\n \tconst char *parent_ident;\n+\tint showed_ident = 0;\n \n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n@@ -582,15 +583,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\t\t\t\t getenv(\"GIT_COMMITTER_EMAIL\")));\n \t\tif (strcmp(author_ident, committer_ident))\n \t\t\tfprintf(fp,\n-\t\t\t\t\"# Author:    %s\\n\",\n+\t\t\t\t\"%s# Author:    %s\\n\",\n+\t\t\t\tshowed_ident++ ? \"\" : \"#\\n\",\n \t\t\t\tfmt_name(author_name, author_email));\n \n-\t\tparent_ident = xstrdup(get_parent_ident());\n-\t\tif (strcmp(parent_ident, committer_ident))\n+\t\tparent_ident = get_parent_ident();\n+\t\tif (!parent_ident || strcmp(parent_ident, committer_ident))\n \t\t\tfprintf(fp,\n-\t\t\t\t\"# Committer: %s\\n\",\n+\t\t\t\t\"%s# Committer: %s\\n\",\n+\t\t\t\tshowed_ident++ ? \"\" : \"#\\n\",\n \t\t\t\tcommitter_ident);\n \n+\t\tif (showed_ident)\n+\t\t\tfprintf(fp, \"#\\n\");\n+\n \t\tsaved_color_setting = wt_status_use_color;\n \t\twt_status_use_color = 0;\n \t\tcommitable = run_status(fp, index_file, prefix, 1);\n-- \n1.5.5.1.183.g593d89.dirty\n"},{"id":"75688","messageId":"20080430163701.GA6544@sigill.intra.peff.net","threadId":"13332","inReplyTo":"20080430145017.GA922@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T16:37:01Z","receivedAt":"2008-04-30T16:37:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2008 at 10:50:17AM -0400, Jeff King wrote:\n\n> A few other comments (I like the idea overall):\n\nActually, thinking about this a bit more (and using it), I'm not sure\nthat the best rule for showing the committer is \"differs from the\nprevious committer.\" For example, if I am building on Junio's git.git,\nthe committer name is shown, even though I have made many commits in\nthis repo already.\n\nSo if the goal is to show the committer name only when it \"matters\", I\ndon't think you have succeeded.\n\n-Peff\n"},{"id":"75703","messageId":"20080430175224.GA4131@steel.home","threadId":"13332","inReplyTo":"1209545236-4266-4-git-send-email-sbejar@gmail.com","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-04-30T17:52:24Z","receivedAt":"2008-04-30T17:52:24Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Santi Béjar, Wed, Apr 30, 2008 10:47:16 +0200:\n> To remind the committer ident in case it is not what you want\n> (taken from the gecos field, want to use different ident in\n> different repositories).\n\nLike: show committer id always if the repo is any kind of a public\nproject. Which maybe ok, I'd better always see what id I use to work\non, for example, Git.\n\nI like the idea in general, but the implementation could be refined.\nMaybe consider looking at user.name (whether the repo has an ident\nset)? This is usually done conciously, and the user wont need this\nreminder.\n"},{"id":"75707","messageId":"8aa486160804301223w574fe425n5d0678020ff1e7b4@mail.gmail.com","threadId":"13332","inReplyTo":"20080430163701.GA6544@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-04-30T19:23:43Z","receivedAt":"2008-04-30T19:23:43Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Wed, Apr 30, 2008 at 6:37 PM, Jeff King <peff@peff.net> wrote:\n>  Actually, thinking about this a bit more (and using it), I'm not sure\n>  that the best rule for showing the committer is \"differs from the\n>  previous committer.\" For example, if I am building on Junio's git.git,\n>  the committer name is shown, even though I have made many commits in\n>  this repo already.\n>\n>  So if the goal is to show the committer name only when it \"matters\", I\n>  don't think you have succeeded.\n>\n>  -Peff\n>\n\nOn Wed, Apr 30, 2008 at 7:52 PM, Alex Riesen <raa.lkml@gmail.com> wrote:\n>  I like the idea in general, but the implementation could be refined.\n>  Maybe consider looking at user.name (whether the repo has an ident\n>  set)? This is usually done conciously, and the user wont need this\n>  reminder.\n\nMaybe only show the committer ident when both happens:\n\n1) the committer ident is different from the parent\n2) the committer ident is set automatically\n\nSanti\n\nP.D.: Jeff, thanks for the amending patch.\n"},{"id":"75708","messageId":"20080430192639.GC30344@sigill.intra.peff.net","threadId":"13332","inReplyTo":"8aa486160804301223w574fe425n5d0678020ff1e7b4@mail.gmail.com","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-04-30T19:26:39Z","receivedAt":"2008-04-30T19:26:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 30, 2008 at 09:23:43PM +0200, Santi Béjar wrote:\n\n> >  I like the idea in general, but the implementation could be refined.\n> >  Maybe consider looking at user.name (whether the repo has an ident\n> >  set)? This is usually done conciously, and the user wont need this\n> >  reminder.\n> \n> Maybe only show the committer ident when both happens:\n> \n> 1) the committer ident is different from the parent\n> 2) the committer ident is set automatically\n\nHonestly, I think just \"2)\" is probably fine (where automatically\npresumably means \"from GECOS\"). I see what you are trying to accomplish\nwith \"1)\", but it's so workflow specific as to be useless.\n\n> P.D.: Jeff, thanks for the amending patch.\n\nNo problem.\n\n-Peff\n"},{"id":"75737","messageId":"8aa486160805010134l4746f9afhec546a9d49fd7a6d@mail.gmail.com","threadId":"13332","inReplyTo":"20080430192639.GC30344@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-05-01T08:34:44Z","receivedAt":"2008-05-01T08:34:44Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Wed, Apr 30, 2008 at 9:26 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Apr 30, 2008 at 09:23:43PM +0200, Santi Béjar wrote:\n>\n>  > >  I like the idea in general, but the implementation could be refined.\n>  > >  Maybe consider looking at user.name (whether the repo has an ident\n>  > >  set)? This is usually done conciously, and the user wont need this\n>  > >  reminder.\n>  >\n>  > Maybe only show the committer ident when both happens:\n>  >\n>  > 1) the committer ident is different from the parent\n>  > 2) the committer ident is set automatically\n>\n>  Honestly, I think just \"2)\" is probably fine (where automatically\n>  presumably means \"from GECOS\"). I see what you are trying to accomplish\n>  with \"1)\", but it's so workflow specific as to be useless.\n\nI don't see \"1) as workflow specific, it is there to minimize the\nnumber of time it is shown, ie somebody might prefer the automatic\nident, so as long as it does not change it is not shown.\n\nSanti\n"},{"id":"75753","messageId":"20080501135109.GA10135@sigill.intra.peff.net","threadId":"13332","inReplyTo":"8aa486160805010134l4746f9afhec546a9d49fd7a6d@mail.gmail.com","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-01T13:51:09Z","receivedAt":"2008-05-01T13:51:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote:\n\n> >  > 1) the committer ident is different from the parent\n> >  > 2) the committer ident is set automatically\n> >\n> >  Honestly, I think just \"2)\" is probably fine (where automatically\n> >  presumably means \"from GECOS\"). I see what you are trying to accomplish\n> >  with \"1)\", but it's so workflow specific as to be useless.\n> \n> I don't see \"1) as workflow specific, it is there to minimize the\n> number of time it is shown, ie somebody might prefer the automatic\n> ident, so as long as it does not change it is not shown.\n\nYes, but whether it actually succeeds in minimizing is dependent on the\nworkflow of the user. I.e., it works great if I tend to build on small,\npersonal repositories (or topic branches), or if I tend to receive other\npeople's work by patches that I commit myself. But if I am pulling from\nsomebody else's repo (as many of us do from git.git), or if I use a\nshared repo with the other committers, then I am very likely to be\nbuilding on a parent that was committed by somebody else (and\nconsequently will see the committer mentioned way more often than was\nintended).\n\n-Peff\n"},{"id":"75774","messageId":"7vd4o5zw2s.fsf@gitster.siamese.dyndns.org","threadId":"13332","inReplyTo":"20080501135109.GA10135@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-01T19:02:51Z","receivedAt":"2008-05-01T19:02:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote:\n>\n>> >  > 1) the committer ident is different from the parent\n>> >  > 2) the committer ident is set automatically\n>> >\n>> >  Honestly, I think just \"2)\" is probably fine (where automatically\n>> >  presumably means \"from GECOS\"). I see what you are trying to accomplish\n>> >  with \"1)\", but it's so workflow specific as to be useless.\n>> \n>> I don't see \"1) as workflow specific, it is there to minimize the\n>> number of time it is shown, ie somebody might prefer the automatic\n>> ident, so as long as it does not change it is not shown.\n>\n> Yes, but whether it actually succeeds in minimizing is dependent on the\n> workflow of the user. I.e., it works great if I tend to build on small,\n> personal repositories (or topic branches), or if I tend to receive other\n> people's work by patches that I commit myself. But if I am pulling from\n> somebody else's repo (as many of us do from git.git), or if I use a\n> shared repo with the other committers, then I am very likely to be\n> building on a parent that was committed by somebody else (and\n> consequently will see the committer mentioned way more often than was\n> intended).\n\nThe goal of the patch as I understand it is to prevent mistakes of\ncommitting under a wrong committer ident, isn't it?  Why does \"minimizing\"\ncome into the picture?\n\nOnce you have a good algorithm to see when to trigger the warning that the\nuser might be using an unintended committer identity, I do not think you\nshould refrain from issuing the warning when you see the offending\ncommitter ident and whose commit you are building on top of should not\naffect it.  Otherwise, the user will get the warning once (or not even get\nthe warning because the commit was made with a \"commit -s -m 'typofix'\"\none liner), and keep using the wrong ident without noticing before\nbuilding up a long chain of commits.  If the identification algorithm is\nbogus, that would result in too many false hits to be annoying, and in\nthat case, (1) I would not apply such a bogus algorithm until it gets\ninto reasonable shape, (2) we would improve it after it gets applied and\nif still makes noise on a hopefully rarer false positive cases, and/or (3)\nthe warning will be accompanied with a suggestion to explicitly use\nuser.name and/or user.email in the configuration file to allow the user to\nsquelch it.\n\nI think the other patch about showing the author when you are committing\nother's changes is a good move, by the way.\n"},{"id":"75786","messageId":"20080501222801.GA21731@sigill.intra.peff.net","threadId":"13332","inReplyTo":"7vd4o5zw2s.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-01T22:28:01Z","receivedAt":"2008-05-01T22:28:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 01, 2008 at 12:02:51PM -0700, Junio C Hamano wrote:\n\n> Once you have a good algorithm to see when to trigger the warning that the\n> user might be using an unintended committer identity, I do not think you\n> should refrain from issuing the warning when you see the offending\n> committer ident and whose commit you are building on top of should not\n> affect it.  Otherwise, the user will get the warning once (or not even get\n\nI think I must not be writing very clearly, because that is basically\nthe same point I have been trying to make in this thread.  Santi's\noriginal algorithm for warning about the wrong committer ident was to\ncheck whether it matched the parent commit. But my point was that is a\nbad algorithm, because it has way too many false positives (i.e., you\nwill end up showing the committer _all the time_ in many workflows).\nSo the followup was \"check this other thing, and also look at whether it\nmatches the parent\" to which my response was \"why bother checking the\nparent match then?\"\n\nI also think you could argue that we should just show the committer all\nthe time. But I don't think anyone has made that argument.\n\n> I think the other patch about showing the author when you are committing\n> other's changes is a good move, by the way.\n\nI agree; it has a very straightforward and sensible algorithm for when\nto show.\n\n-Peff\n"},{"id":"75815","messageId":"8aa486160805020225p47230100m86f9ee251dabc161@mail.gmail.com","threadId":"13332","inReplyTo":"20080501222801.GA21731@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2008-05-02T09:25:38Z","receivedAt":"2008-05-02T09:25:38Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Fri, May 2, 2008 at 12:28 AM, Jeff King <peff@peff.net> wrote:\n> On Thu, May 01, 2008 at 12:02:51PM -0700, Junio C Hamano wrote:\n>\n>  > Once you have a good algorithm to see when to trigger the warning that the\n>  > user might be using an unintended committer identity, I do not think you\n>  > should refrain from issuing the warning when you see the offending\n>  > committer ident and whose commit you are building on top of should not\n>  > affect it.  Otherwise, the user will get the warning once (or not even get\n>\n>  I think I must not be writing very clearly,\n\nIt was clear, but I think there is more than this, especially the\ndefinition of what is a wrong committer depends on the user. I was\ntrying to not annoy too much to some users (those who like the\nautomatic committer).\n\nLet's find the definition of \"wrong committer\":\n\n1) user.{name,email} or GIT_COMMITTER_{NAME,EMAIL} is not a wrong committer.\n2) automatic without a domain name (user@hostname.(none)) is a wrong committer.\n2) automatic or partially set ident:\n    a) wrong committer for some users\n    b) right committer for others\n\nI see different strategies. Show the committer:\n\n1) always\n2) when user.warn = yes (defaulting to yes)\n3) when it is automatic\n    a) always\n    b) and different from parent\n    c) and different from a list of valid committer idents\n    d) and when user.WarnAutomatic = yes (defaulting to yes)\n\n(the names of the configs are just ideas)\n\nI prefer in order: 3a, 3d, 2, 1.\n\n>\n>  I also think you could argue that we should just show the committer all\n>  the time. But I don't think anyone has made that argument.\n\n>From past threads and the \"precious screen state\" I thought it was\ntotally discarded.\n\nSanti\n"}]}