{"thread":{"id":"26953","subject":"[PATCH 1/2] blame: add --abbrev command line option","startedAt":"2011-04-01T01:54:55Z","lastAt":"2011-04-02T01:37:08Z","messageCount":4,"participants":["Namhyung Kim","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"164852","messageId":"1301622896-5836-1-git-send-email-namhyung@gmail.com","threadId":"26953","inReplyTo":null,"subject":"[PATCH 1/2] blame: add --abbrev command line option","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-04-01T01:54:55Z","receivedAt":"2011-04-01T01:54:55Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Signed-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n builtin/blame.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex f6b03f7..253b480 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -41,6 +41,7 @@ static int reverse;\n static int blank_boundary;\n static int incremental;\n static int xdl_opts;\n+static int abbrev = 8;\n \n static enum date_mode blame_date_mode = DATE_ISO8601;\n static size_t blame_date_width;\n@@ -1670,7 +1671,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\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 = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : abbrev;\n \n \t\tif (suspect->commit->object.flags & UNINTERESTING) {\n \t\t\tif (blank_boundary)\n@@ -2310,6 +2311,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t{ OPTION_CALLBACK, 'C', NULL, &opt, \"score\", \"Find line copies within and across files\", PARSE_OPT_OPTARG, blame_copy_callback },\n \t\t{ OPTION_CALLBACK, 'M', NULL, &opt, \"score\", \"Find line movements within and across files\", PARSE_OPT_OPTARG, blame_move_callback },\n \t\tOPT_CALLBACK('L', NULL, &bottomtop, \"n,m\", \"Process only line range n,m, counting from 1\", blame_bottomtop_callback),\n+\t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\n \n-- \n1.7.4\n"},{"id":"164853","messageId":"1301622896-5836-2-git-send-email-namhyung@gmail.com","threadId":"26953","inReplyTo":"1301622896-5836-1-git-send-email-namhyung@gmail.com","subject":"[PATCH 2/2] blame: honor core.abbrev","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-04-01T01:54:56Z","receivedAt":"2011-04-01T01:54:56Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"If user sets config.abbrev option, use it as if --abbrev was given.\nWe can't set abbrev to default_abbrev unconditionally IMHO, because\nhistorically default abbrev length of the blame command is 8 and\nDEFAULT_ABBREV is 7.\n\nAs you may see, this is the default value and user can use different\nabbrev length by specifing --abbrev=N command line option.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n builtin/blame.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 253b480..93693d2 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2317,12 +2317,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \n \tstruct parse_opt_ctx_t ctx;\n \tint cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n+\tint orig_abbrev = DEFAULT_ABBREV;\n \n \tgit_config(git_blame_config, NULL);\n \tinit_revisions(&revs, NULL);\n \trevs.date_mode = blame_date_mode;\n \tDIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);\n \n+\t/* if user sets config.abbrev, honor it */\n+\tif (orig_abbrev != default_abbrev)\n+\t\tabbrev = default_abbrev;\n \tsave_commit_buffer = 0;\n \tdashdash_pos = 0;\n \n-- \n1.7.4\n"},{"id":"164934","messageId":"7vbp0pegmb.fsf@alter.siamese.dyndns.org","threadId":"26953","inReplyTo":"1301622896-5836-2-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH 2/2] blame: honor core.abbrev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-01T22:44:44Z","receivedAt":"2011-04-01T22:44:44Z","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> If user sets config.abbrev option, use it as if --abbrev was given.\n> We can't set abbrev to default_abbrev unconditionally IMHO, because\n> historically default abbrev length of the blame command is 8 and\n> DEFAULT_ABBREV is 7.\n\nIsn't the one-letter difference because we sometimes need to show the\nboundary commit with a caret at the beginning?\n\nI think the way this patch initializes orig_abbrev using DEFAULT_ABBREV is\nwrong (at that point, I don't think you have called git_config() to get\nthe user config for DEFAULT_ABBREV).\n\nSee the patch to describe.c in dce9648 (Make the default abbrev length\nconfigurable, 2010-10-28) for the right way to do this.\n\n - initialize the variable to -1;\n - call git_config() to get correct value in DEFAULT_ABBREV;\n - call parse_options() to potentially update the variable; then\n - if variable is still -1, assign DEFAULT_ABBREV to it.\n\nAfter all that, add 1 to it to account for the possible boundary caret.\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 253b480..93693d2 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -2317,12 +2317,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \n>  \tstruct parse_opt_ctx_t ctx;\n>  \tint cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n> +\tint orig_abbrev = DEFAULT_ABBREV;\n>  \n>  \tgit_config(git_blame_config, NULL);\n>  \tinit_revisions(&revs, NULL);\n>  \trevs.date_mode = blame_date_mode;\n>  \tDIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);\n>  \n> +\t/* if user sets config.abbrev, honor it */\n> +\tif (orig_abbrev != default_abbrev)\n> +\t\tabbrev = default_abbrev;\n>  \tsave_commit_buffer = 0;\n>  \tdashdash_pos = 0;\n"},{"id":"164938","messageId":"1301708228.1507.34.camel@leonhard","threadId":"26953","inReplyTo":"7vbp0pegmb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] blame: honor core.abbrev","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2011-04-02T01:37:08Z","receivedAt":"2011-04-02T01:37:08Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hello, Junio-san.\n\n2011-04-01 (금), 15:44 -0700, Junio C Hamano:\n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > If user sets config.abbrev option, use it as if --abbrev was given.\n> > We can't set abbrev to default_abbrev unconditionally IMHO, because\n> > historically default abbrev length of the blame command is 8 and\n> > DEFAULT_ABBREV is 7.\n> \n> Isn't the one-letter difference because we sometimes need to show the\n> boundary commit with a caret at the beginning?\n> \n\nYeah, I guessed so.\n\n\n> I think the way this patch initializes orig_abbrev using DEFAULT_ABBREV is\n> wrong (at that point, I don't think you have called git_config() to get\n> the user config for DEFAULT_ABBREV).\n> \n\nMy intention was that I want to check whether the user sets the option\nor not and apply the value only if it is set (ig. different from \"7\").\nPlease see below.\n\n\n> See the patch to describe.c in dce9648 (Make the default abbrev length\n> configurable, 2010-10-28) for the right way to do this.\n> \n>  - initialize the variable to -1;\n>  - call git_config() to get correct value in DEFAULT_ABBREV;\n>  - call parse_options() to potentially update the variable; then\n>  - if variable is still -1, assign DEFAULT_ABBREV to it.\n> \n> After all that, add 1 to it to account for the possible boundary caret.\n> \n\nI saw the patch already, and, yes, it would be a simple and probably the\nright way to do. But it could give a bit of confusion to the users IMHO.\nSay, A user specify --abbbrev=10 then he/she might expect 10 hexdigits\nin the output but actually [s]he will get 11.\n\nSo I decided to use the number as is, not adding 1. And this would be a\nconstant behavior from the patch 1/2 and all other git command which use\n--abbrev option, I guess. But I kept to use 8 in case the user doesn't\ngive the config or command-line option, just for backward compatibility.\n\nI'm not sure this is really a concern. Maybe we can give them 11 as you\nsaid and document the behavior in the manual page.\n\nThanks.\n\n\n-- \nRegards,\nNamhyung Kim\n"}]}