{"thread":{"id":"26679","subject":"[PATCH] blame: honor core.abbrevguard","startedAt":"2011-03-07T04:35:40Z","lastAt":"2011-03-08T01:38:16Z","messageCount":4,"participants":["Namhyung Kim","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"162895","messageId":"1299472540-6066-1-git-send-email-namhyung@gmail.com","threadId":"26679","inReplyTo":null,"subject":"[PATCH] blame: honor core.abbrevguard","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-07T04:35:40Z","receivedAt":"2011-03-07T04:35:40Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Commit 72a5b561 (\"core.abbrevguard: Ensure short object names stay\nunique a bit longer\") introduced this config variable to make object\nname unambiguously. Use it.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n builtin/blame.c |   10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex aa30ec52692..d1cf22beba1 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1664,14 +1664,22 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \tstruct commit_info ci;\n \tchar hex[41];\n \tint show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);\n+\tint length;\n \n \tget_commit_info(suspect->commit, &ci, 1);\n \tstrcpy(hex, sha1_to_hex(suspect->commit->object.sha1));\n \n+\tif (opt & OUTPUT_LONG_OBJECT_NAME)\n+\t\tlength = 40;\n+\telse {\n+\t\tlength = 8 + unique_abbrev_extra_length;\n+\t\tif (length > 40)\n+\t\t\tlength = 40;\n+\t}\n+\n \tcp = nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n \t\tchar ch;\n-\t\tint length = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : 8;\n \n \t\tif (suspect->commit->object.flags & UNINTERESTING) {\n \t\t\tif (blank_boundary)\n-- \n1.7.4\n"},{"id":"162897","messageId":"1299475335-8436-1-git-send-email-namhyung@gmail.com","threadId":"26679","inReplyTo":"1299472540-6066-1-git-send-email-namhyung@gmail.com","subject":"[PATCH v2] blame: honor core.abbrevguard","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-07T05:22:15Z","receivedAt":"2011-03-07T05:22:15Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Commit 72a5b561 (\"core.abbrevguard: Ensure short object names stay\nunique a bit longer\") introduced this config variable to make object\nname unambiguously. Use it.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\nv2:\n - re-init 'length' in every loop\n\n builtin/blame.c |   11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex aa30ec52692..cecfceea45f 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1664,14 +1664,23 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \tstruct commit_info ci;\n \tchar hex[41];\n \tint show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);\n+\tint uniq_length;\n \n \tget_commit_info(suspect->commit, &ci, 1);\n \tstrcpy(hex, sha1_to_hex(suspect->commit->object.sha1));\n \n+\tif (opt & OUTPUT_LONG_OBJECT_NAME)\n+\t\tuniq_length = 40;\n+\telse {\n+\t\tuniq_length = 8 + unique_abbrev_extra_length;\n+\t\tif (uniq_length > 40)\n+\t\t\tuniq_length = 40;\n+\t}\n+\n \tcp = nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n \t\tchar ch;\n-\t\tint length = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : 8;\n+\t\tint length = uniq_length;\n \n \t\tif (suspect->commit->object.flags & UNINTERESTING) {\n \t\t\tif (blank_boundary)\n-- \n1.7.4\n"},{"id":"162953","messageId":"7vipvuzau2.fsf@alter.siamese.dyndns.org","threadId":"26679","inReplyTo":"1299475335-8436-1-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH v2] blame: honor core.abbrevguard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-08T00:59:17Z","receivedAt":"2011-03-08T00:59:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> Commit 72a5b561 (\"core.abbrevguard: Ensure short object names stay\n> unique a bit longer\") introduced this config variable to make object\n> name unambiguously. Use it.\n>\n> Signed-off-by: Namhyung Kim <namhyung@gmail.com>\n> ---\n> v2:\n>  - re-init 'length' in every loop\n\nIn this codepath, we only aim to make sure that the shortened object name\nis of the same length.  By choosing hardcoded 8, the original clearly\ndeclares that we do not _care_ about the uniqueness.\n\nThe abbrev-guard that adds to the length that ensures the uniqueness in\nthe particular repository has a very different semantics.  It only makes\nsense if you add to a length that you know would make the object names\nminimally unique.  Adding it to hardcoded 8 does not make it less\nambiguous the same way as the configuration variable intends to do.\n\nSo I am not entirely happy with this patch.\n\nBesides, when OUTPUT_LONG_OBJECT_NAME is specified, the value this\nvariable holds is _not_ \"uniq_length\", so the name of the variable is not\nquite correct.  We colloquially call the object name \"sha1\" in the code,\nso \"sha1_length\" would be a better name for it.\n\nIf we _were_ to do a change that uses the configuration variable, I would\nimagine that it would be a part of a change that makes sure that the\nshortened object names in the output actually uniquely identify the\ncommits.  It would involve find_unique_abbrev() for as many commits as the\nnumber of lines in the blamed range at most (and at that point the abbrev\nguard would automatically taken into account because find_unique_abbrev()\nalready does so) before calling \"output()\"; I suspect that find_alignment()\nis the right function to do so, as that is where the column alignment\nlogic all happens.\n\nIt would probably need to be introduced as a new feature, with its own\ncommand line option, similar to -l option that forces the full 40-hex\noutput.\n\nSo thanks for a patch, but I don't think we would want to take it in the\ncurrent shape.\n"},{"id":"162957","messageId":"1299548296.1467.18.camel@leonhard","threadId":"26679","inReplyTo":"7vipvuzau2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] blame: honor core.abbrevguard","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-03-08T01:38:16Z","receivedAt":"2011-03-08T01:38:16Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hi,\n\n2011-03-07 (월), 16:59 -0800, Junio C Hamano:\n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > Commit 72a5b561 (\"core.abbrevguard: Ensure short object names stay\n> > unique a bit longer\") introduced this config variable to make object\n> > name unambiguously. Use it.\n> >\n> > Signed-off-by: Namhyung Kim <namhyung@gmail.com>\n> > ---\n> > v2:\n> >  - re-init 'length' in every loop\n> \n> In this codepath, we only aim to make sure that the shortened object name\n> is of the same length.  By choosing hardcoded 8, the original clearly\n> declares that we do not _care_ about the uniqueness.\n> \n\nJust out of curiousity, where did the number come from? I guess\nDEFAULT_ABBREV + '^' ?\n\n\n> The abbrev-guard that adds to the length that ensures the uniqueness in\n> the particular repository has a very different semantics.  It only makes\n> sense if you add to a length that you know would make the object names\n> minimally unique.  Adding it to hardcoded 8 does not make it less\n> ambiguous the same way as the configuration variable intends to do.\n> \n> So I am not entirely happy with this patch.\n> \n> Besides, when OUTPUT_LONG_OBJECT_NAME is specified, the value this\n> variable holds is _not_ \"uniq_length\", so the name of the variable is not\n> quite correct.  We colloquially call the object name \"sha1\" in the code,\n> so \"sha1_length\" would be a better name for it.\n> \n\nOK. I was not happy with the name, too. :)\n\n\n> If we _were_ to do a change that uses the configuration variable, I would\n> imagine that it would be a part of a change that makes sure that the\n> shortened object names in the output actually uniquely identify the\n> commits.  It would involve find_unique_abbrev() for as many commits as the\n> number of lines in the blamed range at most (and at that point the abbrev\n> guard would automatically taken into account because find_unique_abbrev()\n> already does so) before calling \"output()\"; I suspect that find_alignment()\n> is the right function to do so, as that is where the column alignment\n> logic all happens.\n> \n> It would probably need to be introduced as a new feature, with its own\n> command line option, similar to -l option that forces the full 40-hex\n> output.\n> \n\nLooks Great. How about -u/--unique? I'm also thinking about --abbrev\ntoo, isn't it useful at all?\n\n\n> So thanks for a patch, but I don't think we would want to take it in the\n> current shape.\n\nThanks for your kindly comment.\n\n\n-- \nRegards,\nNamhyung Kim\n"}]}