{"thread":{"id":"24506","subject":"Possible bug with `export-subst' attribute","startedAt":"2010-07-25T09:08:12Z","lastAt":"2010-07-28T17:23:03Z","messageCount":15,"participants":["Eli Barzilay","Ilari Liusvaara","Jonathan Nieder","Junio C Hamano","Will Palmer"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"146253","messageId":"19531.65276.394443.190317@winooski.ccs.neu.edu","threadId":"24506","inReplyTo":null,"subject":"Possible bug with `export-subst' attribute","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2010-07-25T09:08:12Z","receivedAt":"2010-07-25T09:08:12Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"I have a file with:\n\n (define archive-id \"$Format:%ct|%h|a$\")\n\nand an `export-subst' attribute -- and it looks like the \"%h\" results\nin a full sha1 instead of the abbreviated one when used with `git\narchive'. This is with 1.7..2 -- I'm not sure, but I think that it\nworked fine with 1.7.1.\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"146278","messageId":"20100725130935.GA22083@LK-Perkele-V2.elisa-laajakaista.fi","threadId":"24506","inReplyTo":"19531.65276.394443.190317@winooski.ccs.neu.edu","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-07-25T13:09:35Z","receivedAt":"2010-07-25T13:09:35Z","isPatch":false,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Jul 25, 2010 at 05:08:12AM -0400, Eli Barzilay wrote:\n> I have a file with:\n> \n> (define archive-id \"$Format:%ct|%h|a$\")\n> \n> and an `export-subst' attribute -- and it looks like the \"%h\" results\n> in a full sha1 instead of the abbreviated one when used with `git\n> archive'. This is with 1.7..2 -- I'm not sure, but I think that it\n> worked fine with 1.7.1.\n \nI remember seeing similar stuff. It isn't just archive, I also rember seeing\ncommit printing full hashes in that informational line it prints when it has\nmade the commit (IIRC, normally that hash is abbrevated).\n\n-Ilari\n"},{"id":"146332","messageId":"20100725221539.GA21813@burratino","threadId":"24506","inReplyTo":"20100725130935.GA22083@LK-Perkele-V2.elisa-laajakaista.fi","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-25T22:15:39Z","receivedAt":"2010-07-25T22:15:39Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ilari Liusvaara wrote:\n\n> I remember seeing similar stuff. It isn't just archive, I also rember seeing\n> commit printing full hashes in that informational line it prints when it has\n> made the commit (IIRC, normally that hash is abbrevated).\n\nMy bad. Would something like this fix it?\n\n-- 8< --\nSubject: archive, commit: use --abbrev by default again\n\nv1.7.1.1~17^2~3 (pretty: Respect --abbrev option, 2010-05-03) taught\ngit log --format=%h to respect the --abbrev option instead of\nalways abbreviating, with the side-effect that we have to pay\nattention to the abbrev setting now.\n\nFor example, the \"git archive\" export-subst feature and the\ninformational line printed by \"git commit\" are using unabbreviated\nobject names now, the former because full object names are the\nlow-level default, the latter because it was first written to imitate\nplumbing.\n\nFix them. While at it, remove a similar confusing\nassignment of 0 to rev.abbrev in \"git checkout\" which had\nno effect.\n\nSigned-off-by: Jonathan Nieder \n---\ndiff --git i/archive.c w/archive.c\nindex d700af3..edd6853 100644\n--- i/archive.c\n+++ w/archive.c\n@@ -33,6 +33,7 @@ static void format_subst(const struct commit *commit,\n \tstruct strbuf fmt = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tctx.date_mode = DATE_NORMAL;\n+\tctx.abbrev = DEFAULT_ABBREV;\n \n \tif (src == buf->buf)\n \t\tto_free = strbuf_detach(buf, NULL);\ndiff --git i/builtin/checkout.c w/builtin/checkout.c\nindex 1994be9..eef2b48 100644\n--- i/builtin/checkout.c\n+++ w/builtin/checkout.c\n@@ -279,7 +279,6 @@ static void show_local_changes(struct object *head)\n \tstruct rev_info rev;\n \t/* I think we want full paths, even if we're in a subdirectory. */\n \tinit_revisions(&rev, NULL);\n-\trev.abbrev = 0;\n \trev.diffopt.output_format |= DIFF_FORMAT_NAME_STATUS;\n \tif (diff_setup_done(&rev.diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\ndiff --git i/builtin/commit.c w/builtin/commit.c\nindex a78dbd8..ae4831e 100644\n--- i/builtin/commit.c\n+++ w/builtin/commit.c\n@@ -1163,7 +1163,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \n-\trev.abbrev = 0;\n+\trev.abbrev = DEFAULT_ABBREV;\n \trev.diff = 1;\n \trev.diffopt.output_format =\n \t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n-- \n"},{"id":"146334","messageId":"19532.48534.761164.164123@winooski.ccs.neu.edu","threadId":"24506","inReplyTo":"20100725221539.GA21813@burratino","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2010-07-25T22:41:26Z","receivedAt":"2010-07-25T22:41:26Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"On Jul 25, Jonathan Nieder wrote:\n> Ilari Liusvaara wrote:\n> \n> > I remember seeing similar stuff. It isn't just archive, I also rember seeing\n> > commit printing full hashes in that informational line it prints when it has\n> > made the commit (IIRC, normally that hash is abbrevated).\n> \n> My bad. Would something like this fix it?\n\nIn my case (using archive), this fixes it -- thanks!\n\n-- \n ((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"146346","messageId":"7vbp9uaii2.fsf@alter.siamese.dyndns.org","threadId":"24506","inReplyTo":"20100725221539.GA21813@burratino","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-26T06:01:57Z","receivedAt":"2010-07-26T06:01:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> My bad. Would something like this fix it?\n>\n> -- 8< --\n> Subject: archive, commit: use --abbrev by default again\n>\n> v1.7.1.1~17^2~3 (pretty: Respect --abbrev option, 2010-05-03) taught\n> git log --format=%h to respect the --abbrev option instead of\n> always abbreviating, with the side-effect that we have to pay\n> attention to the abbrev setting now.\n>\n> For example, the \"git archive\" export-subst feature and the\n> informational line printed by \"git commit\" are using unabbreviated\n> object names now, the former because full object names are the low-level\n> default, the latter because it was first written to imitate plumbing.\n>\n> Fix them. While at it, remove a similar confusing assignment of 0 to\n> rev.abbrev in \"git checkout\" which had no effect.\n>\n> Signed-off-by: Jonathan Nieder \n\nThe ones to archive and checkout I understand, but what effect does the\none to commit.c::print_summary() have?\n\n> diff --git i/builtin/commit.c w/builtin/commit.c\n> index a78dbd8..ae4831e 100644\n> --- i/builtin/commit.c\n> +++ w/builtin/commit.c\n> @@ -1163,7 +1163,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n> \tinit_revisions(&rev, prefix);\n> \tsetup_revisions(0, NULL, &rev, NULL);\n> \n> -\trev.abbrev = 0;\n> +\trev.abbrev = DEFAULT_ABBREV;\n> \trev.diff = 1;\n> \trev.diffopt.output_format =\n> \t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n"},{"id":"146421","messageId":"20100726190448.GA32367@burratino","threadId":"24506","inReplyTo":"7vbp9uaii2.fsf@alter.siamese.dyndns.org","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-26T19:04:48Z","receivedAt":"2010-07-26T19:04:48Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The ones to archive and checkout I understand, but what effect does the\n> one to commit.c::print_summary() have?\n\nCurrently commit.c::print_summary() does this:\n\n\tstruct strbuf format = STRBUF_INIT;\n\t...\n\n\tstrbuf_addstr(&format, \"format:%h] %s\");\n\t[+ other bits for the commit notice]\n\n\trev.abbrev = 0;\n\trev.diff = 1;\n\t...\n\tget_commit_format(format.buf, &rev)\n\t...\n\n\tprintf(\"[%s%s \",\n\t\t\t[branch name \" (root-commit)\"]);\n\n\tif (!log_tree_commit(&rev, commit)) {\n\t\t...\n\nIn other words, it imbues rev with a format including %h and uses that\nto print a commit summary.\n\nThat code is as old as builtin commit (v1.5.4-rc0~78^2~30,\n2007-11-08) and was meant to imitate a diff-tree invocation (which\nis plumbing).\n\n-- %< --\nSubject: examples/commit: use --abbrev for commit summary\n\nAfter v1.7.1.1~17^2~3 (pretty: Respect --abbrev option, 2010-05-03),\nplumbing users do not abbreviate %h hashes by default any more.\n\nSigned-off-by: Jonathan Nieder \n---\n If this seems to be a problem elsewhere, we will have to decouple\n the remembered --abbrev setting for %h from that for --raw output.\n\ndiff --git i/contrib/examples/git-commit.sh w/contrib/examples/git-commit.sh\nindex 5c72f65..23ffb02 100755\n--- i/contrib/examples/git-commit.sh\n+++ w/contrib/examples/git-commit.sh\n@@ -631,7 +631,7 @@ then\n \tif test -z \"$quiet\"\n \tthen\n \t\tcommit=`git diff-tree --always --shortstat --pretty=\"format:%h: %s\"\\\n-\t\t --summary --root HEAD --`\n+\t\t --abbrev --summary --root HEAD --`\n \t\techo \"Created${initial_commit:+ initial} commit $commit\"\n \tfi\n fi\n-- \n"},{"id":"146498","messageId":"7vzkxc7rpn.fsf@alter.siamese.dyndns.org","threadId":"24506","inReplyTo":"20100726190448.GA32367@burratino","subject":"Re: Possible bug with `export-subst' attribute","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-27T17:35:48Z","receivedAt":"2010-07-27T17:35:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> The ones to archive and checkout I understand, but what effect does the\n>> one to commit.c::print_summary() have?\n>\n> Currently commit.c::print_summary() does this:\n> ...\n> \tif (!log_tree_commit(&rev, commit)) {\n> \t\t...\n>\n> In other words, it imbues rev with a format including %h and uses that\n> to print a commit summary.\n\nSorry, but I think I understood that part.\n\nBut the thing is, we do not seem to show non-abbreviated string there with\nor without your patch, because inside log_tree_diff_flush() -> show_log()\ncallchain we use opt->diffopt.abbrev to decide what is done for that %h\ntoken:\n\n\tctx.abbrev = opt->diffopt.abbrev;\n\nso just like the confusing assignment in builtin/checkout.c, isn't\nthis one in builtin/commit.c also a confusing no-op?\n\nPerhaps I am missing something obvious?\n"},{"id":"146503","messageId":"20100727182942.GB5578@burratino","threadId":"24506","inReplyTo":"7vzkxc7rpn.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] archive: abbreviate substituted commit ids again","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T18:29:42Z","receivedAt":"2010-07-27T18:29:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> isn't\n> this one in builtin/commit.c also a confusing no-op?\n\nI see. When setup_revisions is called (which initializes diffopt),\nrev.abbrev still equals DEFAULT_ABBREV.\n\nI missed v1.7.1.1~17^2 (commit::print_summary(): don't use\nformat_commit_message(), 2010-06-12) and did not notice that the bug had\ngone away. Sorry for the confusion.\n\nHere’s a rerolled series.\n\nJonathan Nieder (3):\n archive: abbreviate substituted commit ids again\n checkout, commit: remove confusing assignments to rev.abbrev\n examples/commit: use --abbrev for commit summary\n\n archive.c | 1 +\n builtin/checkout.c | 1 -\n builtin/commit.c | 1 -\n contrib/examples/git-commit.sh | 2 +-\n t/t5001-archive-attr.sh | 2 +-\n 5 files changed, 3 insertions(+), 4 deletions(-)\n\n-- \n1.7.2.21.g04ff\n"},{"id":"146504","messageId":"20100727183236.GC5578@burratino","threadId":"24506","inReplyTo":"20100727182942.GB5578@burratino","subject":"[PATCH 1/3] archive: abbreviate substituted commit ids again","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T18:32:36Z","receivedAt":"2010-07-27T18:32:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Given a file with:\n\n (define archive-id \"$Format:%ct|%h|a$\")\n\nand an export-subst attribute, the \"%h\" results in an full 40-digit\nobject name instead of the expected 7-digit one.\n\nThe export-subst feature requests unabbreviated object names because\nthat is the low-level default. The effect was not observable until\nv1.7.1.1~17^2~3 (2010-05-03), which taught log --format=%h to respect\nthe --abbrev option.\n\nReported-by: Eli Barzilay \nTested-by: Eli Barzilay \nSigned-off-by: Jonathan Nieder \n---\nI carried over the tested-by; I hope that’s okay. Well, I’ve tested\nthe new patch myself, at least. :)\n\n archive.c | 1 +\n t/t5001-archive-attr.sh | 2 +-\n 2 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex d700af3..edd6853 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -33,6 +33,7 @@ static void format_subst(const struct commit *commit,\n \tstruct strbuf fmt = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tctx.date_mode = DATE_NORMAL;\n+\tctx.abbrev = DEFAULT_ABBREV;\n \n \tif (src == buf->buf)\n \t\tto_free = strbuf_detach(buf, NULL);\ndiff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh\nindex 426b319..02d4d22 100755\n--- a/t/t5001-archive-attr.sh\n+++ b/t/t5001-archive-attr.sh\n@@ -4,7 +4,7 @@ test_description='git archive attribute tests'\n \n . ./test-lib.sh\n \n-SUBSTFORMAT=%H%n\n+SUBSTFORMAT='%H (%h)%n'\n \n test_expect_exists() {\n \ttest_expect_success \" $1 exists\" \"test -e $1\"\n-- \n1.7.2.21.g04ff\n"},{"id":"146507","messageId":"20100727183706.GD5578@burratino","threadId":"24506","inReplyTo":"20100727182942.GB5578@burratino","subject":"[PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T18:37:07Z","receivedAt":"2010-07-27T18:37:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Since they do not precede setup_revisions, these assignments of 0 to\nrev.abbrev have no effect.\n\nv1.7.1.1~17^2~3 (2010-05-03) taught the log --format=%h machinery\nto respect --abbrev instead of always abbreviating, so we have to pay\nattention to the abbrev setting now.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Jonathan Nieder \n---\n builtin/checkout.c | 1 -\n builtin/commit.c | 1 -\n 2 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 1994be9..eef2b48 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -279,7 +279,6 @@ static void show_local_changes(struct object *head)\n \tstruct rev_info rev;\n \t/* I think we want full paths, even if we're in a subdirectory. */\n \tinit_revisions(&rev, NULL);\n-\trev.abbrev = 0;\n \trev.diffopt.output_format |= DIFF_FORMAT_NAME_STATUS;\n \tif (diff_setup_done(&rev.diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a78dbd8..279cfc1 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1163,7 +1163,6 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \tinit_revisions(&rev, prefix);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \n-\trev.abbrev = 0;\n \trev.diff = 1;\n \trev.diffopt.output_format =\n \t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n-- \n1.7.2.21.g04ff\n"},{"id":"146509","messageId":"20100727184450.GE5578@burratino","threadId":"24506","inReplyTo":"20100727182942.GB5578@burratino","subject":"[PATCH 3/3] examples/commit: use --abbrev for commit summary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T18:44:51Z","receivedAt":"2010-07-27T18:44:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"After v1.7.1.1~17^2~3 (pretty: Respect --abbrev option, 2010-05-03),\nplumbing users do not abbreviate %h hashes by default any more.\n\nNoticed while investigating the bug fixed by v1.7.1.1~17^2\n(commit::print_summary(): don't use format_commit_message(),\n2010-06-12).\n\nCc: Will Palmer \nSigned-off-by: Jonathan Nieder \n---\nAgain, sorry for the trouble.\n\nMaybe diff-tree should always abbreviate the format %h. That would\nmake the Parents: line and %p format produce different result when\nthe --abbrev option is not supplied.\n\nI could go either way.\n\n contrib/examples/git-commit.sh | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/examples/git-commit.sh b/contrib/examples/git-commit.sh\nindex 5c72f65..23ffb02 100755\n--- a/contrib/examples/git-commit.sh\n+++ b/contrib/examples/git-commit.sh\n@@ -631,7 +631,7 @@ then\n \tif test -z \"$quiet\"\n \tthen\n \t\tcommit=`git diff-tree --always --shortstat --pretty=\"format:%h: %s\"\\\n-\t\t --summary --root HEAD --`\n+\t\t --abbrev --summary --root HEAD --`\n \t\techo \"Created${initial_commit:+ initial} commit $commit\"\n \tfi\n fi\n-- \n1.7.2.21.g04ff\n"},{"id":"146519","messageId":"1280261936.4462.6.camel@walleee","threadId":"24506","inReplyTo":"20100727183706.GD5578@burratino","subject":"Re: [PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-07-27T20:18:56Z","receivedAt":"2010-07-27T20:18:56Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Tue, 2010-07-27 at 13:37 -0500, Jonathan Nieder wrote:\n...\n> v1.7.1.1~17^2~3 (2010-05-03) taught the log --format=%h machinery\n> to respect --abbrev instead of always abbreviating ...\n...\n\nI've been seeing this phrasing throughout this discussion, and at first\nI thought it was merely a poor choice of words, but now I feel I must\nensure it's clear:\nthe purpose of the patch was to respect --abbrev instead of always\nabbreviating to a minimum of 7 characters. /Not/ to respect abbrev\n\"instead of always abbreviating\". Perhaps armed with that phrasing, a\nmore general solution, such as equating \"0\" with \"DEFAULT_ABBREV\" rather\nthan \"no abbrev\", could be applied?\n\n-- \n-- Will\n"},{"id":"146524","messageId":"20100727210908.GA11317@burratino","threadId":"24506","inReplyTo":"1280261936.4462.6.camel@walleee","subject":"Re: [PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-27T21:09:08Z","receivedAt":"2010-07-27T21:09:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Will Palmer wrote:\n\n> the purpose of the patch was to respect --abbrev instead of always\n> abbreviating to a minimum of 7 characters. /Not/ to respect abbrev\n> \"instead of always abbreviating\".\n\nSure, though it had that added effect.\n\nOne goal of that series was to be able to write formats like this:\n\n\t%C(commit)commit %H%Creset\n\t%M(Merge: %p\n\t)Author: %an <%ae>\n\tDate: %ad\n\n\t%w(0,4,4)%B\n\nto replicate the effect of --format=medium. With diff-tree (and\nrev-list before v1.7.0.6~1^2) that is not possible if %p abbreviates\nby default.\n\nOf course, v1.7.0.6~1^2 illustrates that no one seems to have been\nrelying on the format of Merge: lines, anyway, so I am not saying that\nto make diff-tree --format=medium abbreviate by default would be a bad\nchange.\n\n> Perhaps armed with that phrasing, a\n> more general solution, such as equating \"0\" with \"DEFAULT_ABBREV\" rather\n> than \"no abbrev\", could be applied?\n\nMaybe. If so, one would have to deal with the other callers that\nexplicitly set abbrev to 0.\n\n probably just confusing no-ops:\n\n - bisect.c::bisect_rev_setup\n - bisect.c::show_diff_tree (to imitate diff-tree: probably a no-op\n because there is no setup_revisions call)\n\n means FULL_SHA1:\n\n - diff-files.c::cmd_diff_files\n - diff-index.c::cmd_diff_index\n - diff-tree.c:cmd_diff_tree\n - revision.c::handle_revision_opt\n\nHope that helps,\nJonathan\n"},{"id":"146604","messageId":"1280311304.2378.64.camel@wpalmer.simply-domain","threadId":"24506","inReplyTo":"20100727210908.GA11317@burratino","subject":"Re: [PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2010-07-28T10:01:44Z","receivedAt":"2010-07-28T10:01:44Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Tue, 2010-07-27 at 16:09 -0500, Jonathan Nieder wrote:\n> Will Palmer wrote:\n> \n> > the purpose of the patch was to respect --abbrev instead of always\n> > abbreviating to a minimum of 7 characters. /Not/ to respect abbrev\n> > \"instead of always abbreviating\".\n> \n> Sure, though it had that added effect.\n> \n> One goal of that series was to be able to write formats like this:\n> \n> \t%C(commit)commit %H%Creset\n> \t%M(Merge: %p\n> \t)Author: %an <%ae>\n> \tDate: %ad\n> \n> \t%w(0,4,4)%B\n> \n> to replicate the effect of --format=medium. With diff-tree (and\n> rev-list before v1.7.0.6~1^2) that is not possible if %p abbreviates\n> by default.\n\nNot sure I understand what you're saying here.\n\nHowever, just to note and throw a little unfinished code around: while\nthe series it came from was indeed intended to allow user-defined\nformats which exactly match the output of built-in formats, it was one\nof the few \"useful enough on its own\" patches which got sent along prior\nto the main work being finished. The patch to allow user-defined formats\nwhich match built-ins exactly is much more complicated (probably much\nmore complicated than it needs to be) and leaves plenty of wiggle-room\nfor minor differences in formats.\n\nIt's currently stalled (mostly due to lack of time to think about git,\ncombined with most of my git-related time being spent thinking about\ngit-remote-svn) but the very-unfinished very-broken\nvery-doesn't-do-enough-to-justify-itself proof-of-concept can be found\nhere:\n\nhttp://repo.or.cz/w/git/wpalmer.git/shortlog/refs/heads/pretty/parse-format-poc\n\nor (specific commit) here:\nhttp://repo.or.cz/w/git/wpalmer.git/commit/eac1527aaf7a839bb7b60ed66a7da502b890e8b0\n\n> \n> Of course, v1.7.0.6~1^2 illustrates that no one seems to have been\n> relying on the format of Merge: lines, anyway, so I am not saying that\n> to make diff-tree --format=medium abbreviate by default would be a bad\n> change.\n\nI expect that no one should be relying in scripts on the format of\nanything log produces which is not specified explicitly. For example,\nI'd expect any script which wanted the information --format=medium\nprovides to do so by listing out an explicit format-line such as the one\nyou gave.\n\n> \n> > Perhaps armed with that phrasing, a\n> > more general solution, such as equating \"0\" with \"DEFAULT_ABBREV\" rather\n> > than \"no abbrev\", could be applied?\n> \n> Maybe. If so, one would have to deal with the other callers that\n> explicitly set abbrev to 0.\n\nDoing a simple grep for \"abbrev\" shows many places using \"0\" to mean \"no\nabbreviation\" while many other places use \"40\" to mean \"no\nabbreviation\". That seems bad enough, especially considering --abbrev=0\nwill wind up setting abbrev to MINIMUM_ABBREV.\n\nHere's what I propose:\n - #define NO_ABBREV 40\n - replace all instances of revs->abbrev = 40 and revs->abbrev = 0 with\nrevs->abbrev = NO_ABBREV\n\nThat will at least make it explicit and consistent.\n\nMeanwhile, what does abbrev = 0 actually mean? Once abbrev = 0 has never\nbeen explicitly set, its meaning becomes obvious: undefined. And an\nundefined value should (I think obviously) be interpreted as\nDEFAULT_ABBREV, since that's what the word \"DEFAULT\" actually comes\nfrom. I think it's safe to assume that no code-paths which explicitly\nwant an unabbreviated value (eg: %H) will actually bother to call\nfind_unique_abbrev, especially without explicitly setting either\nrevs->abbrev = 0 (that would be revs->abbrev = NO_ABBREV) or\nrevs->abbrev = 40 (same here)\n\n> Hope that helps,\n> Jonathan\n\n-- Will\n"},{"id":"146640","messageId":"7vhbjj5xmw.fsf@alter.siamese.dyndns.org","threadId":"24506","inReplyTo":"1280311304.2378.64.camel@wpalmer.simply-domain","subject":"Re: [PATCH 2/3] checkout, commit: remove confusing assignments to rev.abbrev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-28T17:23:03Z","receivedAt":"2010-07-28T17:23:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will Palmer writes:\n\n> Here's what I propose:\n> - #define NO_ABBREV 40\n> - replace all instances of revs->abbrev = 40 and revs->abbrev = 0 with\n> revs->abbrev = NO_ABBREV\n>\n> That will at least make it explicit and consistent.\n\nThat is a good idea. I think abbrev == 0 in the early days used to mean\n\"use the compiled-in default, whatever it is\" but somehow some codepaths\nmistakenly used it to mean \"please do not abbreviate\" (my fault).\n\n> ... And an\n> undefined value should (I think obviously) be interpreted as\n> DEFAULT_ABBREV, since that's what the word \"DEFAULT\" actually comes\n> from.\n\nWe would probably need to be a bit careful here. By default plumbing\ncommands do not abbreviate, while we do want the default abbreviation\nin our Porcelains.\n"}]}