{"thread":{"id":"27352","subject":"[PATCH] Add log.abbrev-commit config option","startedAt":"2011-05-14T17:22:38Z","lastAt":"2011-05-18T01:05:23Z","messageCount":22,"participants":["Jay Soffian","Jonathan Nieder","Junio C Hamano","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"167853","messageId":"1305393758-95432-1-git-send-email-jaysoffian@gmail.com","threadId":"27352","inReplyTo":null,"subject":"[PATCH] Add log.abbrev-commit config option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T17:22:38Z","receivedAt":"2011-05-14T17:22:38Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Add log.abbrev-commit (and log.abbrevCommit synonym) config option as\na convenience for users who often use --abbrev-commit with git log and\ngit show (but not git whatchanged, as its output is more likely to be\nparsed even though it is not technically plumbing).\n\nAllow the option to be overridden via --no-abbrev-commit.\n\n(Also, a drive-by spelling correction in git log's short help.)\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\n Documentation/config.txt  |    6 ++++++\n Documentation/git-log.txt |    3 +++\n builtin/log.c             |   17 +++++++++++++++--\n t/t4202-log.sh            |    8 ++++++++\n 4 files changed, 32 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 285c7f73ca..85f00f6bb1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1314,6 +1314,12 @@ interactive.singlekey::\n \tlinkgit:git-checkout[1]. Note that this setting is silently\n \tignored if portable keystroke input is not available.\n \n+log.abbrev-commit::\n+log.abbrevCommit::\n+\tIf true, act as if --abbrev-commit were specified on the command\n+\tline. May be overridden with --no-abbrev-commit. Note that this setting\n+\tis ignored by rev-list.\n+\n log.date::\n \tSet the default date-time mode for the 'log' command.\n \tSetting a value for log.date is similar to using 'git log''s\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex de5c0d37a5..3061eec9b7 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -38,6 +38,9 @@ OPTIONS\n \tContinue listing the history of a file beyond renames\n \t(works only for a single file).\n \n+--no-abbrev-commit::\n+\tDon't abbreviate commit name. Useful for overridding log.abbrevCommit.\n+\n --no-decorate::\n --decorate[=short|full|no]::\n \tPrint out the ref names of any commits that are shown. If 'short' is\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f6219909a7..cfac510b24 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -23,6 +23,7 @@\n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n \n+static int default_abbrev_commit = 0;\n static int default_show_root = 1;\n static int decoration_style;\n static int decoration_given;\n@@ -77,6 +78,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \t\tget_commit_format(fmt_pretty, rev);\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n+\trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n@@ -89,11 +91,13 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n {\n \tstruct userformat_want w;\n-\tint quiet = 0, source = 0;\n+\tint quiet = 0, source = 0, no_abbrev_commit = 0;\n \n \tconst struct option builtin_log_options[] = {\n-\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n+\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"suppress diff output\"),\n \t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n+\t\tOPT_BOOLEAN(0, \"no-abbrev-commit\", &no_abbrev_commit,\n+\t\t\t    \"don't abbreviate commit name\"),\n \t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n \t\t  PARSE_OPT_OPTARG, decorate_callback},\n \t\tOPT_END()\n@@ -129,6 +133,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tif (source)\n \t\trev->show_source = 1;\n \n+\tif (no_abbrev_commit)\n+\t\trev->abbrev_commit = 0;\n+\n \t/*\n \t * defeat log.decorate configuration interacting with --pretty=raw\n \t * from the command line.\n@@ -323,6 +330,11 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcasecmp(var, \"log.abbrevcommit\") ||\n+\t    !strcasecmp(var, \"log.abbrev-commit\")) {\n+\t\tdefault_abbrev_commit = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"log.date\"))\n \t\treturn git_config_string(&default_date_mode, var, value);\n \tif (!strcmp(var, \"log.decorate\")) {\n@@ -347,6 +359,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \tstruct setup_revision_opt opt;\n \n \tgit_config(git_log_config, NULL);\n+\tdefault_abbrev_commit = 0;\n \n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 2fcc31a6f3..83c6576feb 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -450,6 +450,14 @@ test_expect_success 'log.decorate configuration' '\n \n '\n \n+test_expect_success 'log.abbrev-commit configuration' '\n+\ttest_might_fail git config --remove-section log &&\n+\tgit log --abbrev-commit >expect &&\n+\tgit config log.abbrev-commit true &&\n+\tgit log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'show added path under \"--follow -M\"' '\n \t# This tests for a regression introduced in v1.7.2-rc0~103^2~2\n \ttest_create_repo regression &&\n-- \n1.7.5\n"},{"id":"167854","messageId":"BANLkTi=MB5nz7wGkPw6_cf1barPbdN3UAQ@mail.gmail.com","threadId":"27352","inReplyTo":"1305393758-95432-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] Add log.abbrev-commit config option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T17:26:56Z","receivedAt":"2011-05-14T17:26:56Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, May 14, 2011 at 1:22 PM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> +log.abbrev-commit::\n> +log.abbrevCommit::\n> +       If true, act as if --abbrev-commit were specified on the command\n> +       line. May be overridden with --no-abbrev-commit. Note that this setting\n> +       is ignored by rev-list.\n> +\n\nThe last sentence should probably read:\n\n   Honored by the 'log' and 'show' commands.\n\nj.\n"},{"id":"167859","messageId":"20110514190122.GA16851@elie","threadId":"27352","inReplyTo":"1305393758-95432-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] Add log.abbrev-commit config option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-14T19:01:22Z","receivedAt":"2011-05-14T19:01:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nI like the idea.\n\nJay Soffian wrote:\n\n> Add log.abbrev-commit (and log.abbrevCommit synonym)\n\nWhy two options?  It would be more conventional to provide just\nlog.abbrevCommit.  The existing \"[add] ignore-errors\" is explained in\nthe manual to be a mistake and \"[add] ignoreerrors\" is the fixed\nversion; adding more configuration variables with dashes in the name\nwould seem to make guessing variable names even more hit-or-miss.\n\n> config option as\n> a convenience for users who often use --abbrev-commit with git log and\n> git show (but not git whatchanged, as its output is more likely to be\n> parsed even though it is not technically plumbing).\n\nHm, that wouldn't have been my hunch.  Are you aware of any scripts\nthat parse \"git whatchanged\" output?\n\nMore worrying is \"git log --format=raw\".  I think as long as we're\ncautious about rolling this out slowly and noticing breakage early it\nshould be okay.  It might even be nice to find out if there are\nscripts or tests that care deeply about \"git log\"'s output format\n(which would be more reliable if they had been written to use\n\"git rev-list | git diff-tree -s --stdin\").\n\n> Allow the option to be overridden via --no-abbrev-commit.\n\nGood idea anyway.  Once parse_revision_opt learns to use parse_options\nthese negated options would be automatic (though that's a long way\naway).\n\nUnfortunately this wouldn't help scripts much until the option has\nbeen around for a while.  Maybe it would be safer to have two patches\n--- one to add --no-abbrev-commit which could be included in \"maint\"\nand widely deployed, and one to add the new configuration only after\n--no-abbrev-commit can be relied on?  But on the other hand, scripts\ncan be updated today to use rev-list | diff-tree, so maybe that's not\nworth the trouble.\n\nPeople using git by hand would certainly appreciate\n--no-abbrev-commit, I suspect.  Thanks for thinking about these\nthings.\n\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1314,6 +1314,12 @@ interactive.singlekey::\n>  \tlinkgit:git-checkout[1]. Note that this setting is silently\n>  \tignored if portable keystroke input is not available.\n>  \n> +log.abbrev-commit::\n> +log.abbrevCommit::\n> +\tIf true, act as if --abbrev-commit were specified on the command\n> +\tline. May be overridden with --no-abbrev-commit. Note that this setting\n> +\tis ignored by rev-list.\n\nStyle: most of that page is written from the point of view of the\nuser, like:\n\n\tTells 'git apply' how to handle whitespace.  Set this to\n\t\"ignore\" if you don't want to be bothered.  See\n\tlinkgit:git-apply[1] for details.\n\nSo maybe something like:\n\n\tWhether to abbreviate hexadecimal commit object names in\n\toutput from the 'log' family of commands.  The number of\n\tdigits shown is determined by the `--abbrev` command-line\n\toption and `core.abbrev` configuration variable.  Can be\n\toverridden on the command line by --abbrev-commit /\n\t--no-abbrev-commit.  The default is false.\n +\n This does not affect the 'git diff-tree' and 'git rev-list'\n commands.\n\n> --- a/Documentation/git-log.txt\n> +++ b/Documentation/git-log.txt\n> @@ -38,6 +38,9 @@ OPTIONS\n>  \tContinue listing the history of a file beyond renames\n>  \t(works only for a single file).\n>  \n> +--no-abbrev-commit::\n> +\tDon't abbreviate commit name. Useful for overridding log.abbrevCommit.\n\nAlso useful for overriding --abbrev-commit from aliases. :)\n\nShouldn't it be documented next to --abbrev-commit?\n\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -23,6 +23,7 @@\n>  /* Set a default date-time format for git log (\"log.date\" config variable) */\n>  static const char *default_date_mode = NULL;\n>  \n> +static int default_abbrev_commit = 0;\n>  static int default_show_root = 1;\n>  static int decoration_style;\n\nStyle: we try to avoid unnecessary zero initializers for variables in\nthe BSS section.\n\n[...]\n> @@ -89,11 +91,13 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n>  {\n>  \tstruct userformat_want w;\n> -\tint quiet = 0, source = 0;\n> +\tint quiet = 0, source = 0, no_abbrev_commit = 0;\n>  \n>  \tconst struct option builtin_log_options[] = {\n> -\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n> +\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"suppress diff output\"),\n>  \t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n> +\t\tOPT_BOOLEAN(0, \"no-abbrev-commit\", &no_abbrev_commit,\n> +\t\t\t    \"don't abbreviate commit name\"),\n\nWhat happens if I do\n\n\tgit log --no-abbrev-commit --abbrev-commit\n\n?  How about\n\n\tgit log --no-abbrev-commit --no-no-abbrev-commit --abbrev-commit\n\n? :)  The behavior should be nicer if this is implemented in revision.c.\n\n[...]\n> @@ -323,6 +330,11 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>  \t\treturn git_config_string(&fmt_pretty, var, value);\n>  \tif (!strcmp(var, \"format.subjectprefix\"))\n>  \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n> +\tif (!strcasecmp(var, \"log.abbrevcommit\") ||\n> +\t    !strcasecmp(var, \"log.abbrev-commit\")) {\n\nNo need to use strcasecmp --- the vars passed to config functions\nalready have the section and variable names in lowercase.\n\n> @@ -347,6 +359,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n>  \tstruct setup_revision_opt opt;\n>  \n>  \tgit_config(git_log_config, NULL);\n> +\tdefault_abbrev_commit = 0;\n\nPuzzling as mentioned above.\n\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -450,6 +450,14 @@ test_expect_success 'log.decorate configuration' '\n>  \n>  '\n>  \n> +test_expect_success 'log.abbrev-commit configuration' '\n> +\ttest_might_fail git config --remove-section log &&\n> +\tgit log --abbrev-commit >expect &&\n> +\tgit config log.abbrev-commit true &&\n> +\tgit log >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nTo avoid polluting the configuration, it would be nicest to do:\n\n\tgit config log.abbrev-commit true &&\n\ttest_when_finished \"git config --unset log.abbrev-commit\" &&\n\tgit log >actual &&\n\nthough it looks like some tests already protect themselves.\n\nJust because I'm curious: what happens if you do\n\n\tgit config log.abbrev-commit true\n\nin test_create_repo in test-lib.sh?  (I.e., are there many tests that\nwould be confused by this?)  Tests tend to be more picky than user\nscripts about the output of git but it might still be an ok way to\nget a vague sense of the impact.\n\nHope that helps, and thanks for a pleasant read.\n\nRegards,\nJonathan\n"},{"id":"167861","messageId":"BANLkTi=MSfRhUhY1jkRC0agQNp7WHDG9FQ@mail.gmail.com","threadId":"27352","inReplyTo":"20110514190122.GA16851@elie","subject":"Re: [PATCH] Add log.abbrev-commit config option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T19:35:02Z","receivedAt":"2011-05-14T19:35:02Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, May 14, 2011 at 3:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Why two options?  It would be more conventional to provide just\n> log.abbrevCommit.  The existing \"[add] ignore-errors\" is explained in\n> the manual to be a mistake\n\nAh, okay.\n\n>> config option as\n>> a convenience for users who often use --abbrev-commit with git log and\n>> git show (but not git whatchanged, as its output is more likely to be\n>> parsed even though it is not technically plumbing).\n>\n> Hm, that wouldn't have been my hunch.  Are you aware of any scripts\n> that parse \"git whatchanged\" output?\n\nYes, I've seen it in post-receive hooks, however, that's typically in\nbare repos where I suppose it's unlikely for the user to set\nlog.abbrevCommit, so maybe I was just being overly cautious.\n\n> More worrying is \"git log --format=raw\".  I think as long as we're\n> cautious about rolling this out slowly and noticing breakage early it\n> should be okay.  It might even be nice to find out if there are\n> scripts or tests that care deeply about \"git log\"'s output format\n> (which would be more reliable if they had been written to use\n> \"git rev-list | git diff-tree -s --stdin\").\n>\n>> Allow the option to be overridden via --no-abbrev-commit.\n>\n> Good idea anyway.  Once parse_revision_opt learns to use parse_options\n> these negated options would be automatic (though that's a long way\n> away).\n>\n> Unfortunately this wouldn't help scripts much until the option has\n> been around for a while.  Maybe it would be safer to have two patches\n> --- one to add --no-abbrev-commit which could be included in \"maint\"\n> and widely deployed, and one to add the new configuration only after\n> --no-abbrev-commit can be relied on?  But on the other hand, scripts\n> can be updated today to use rev-list | diff-tree, so maybe that's not\n> worth the trouble.\n\nI think that's overkill. Thinking through it more, this would only\nbreak any scripts:\n\na) that are parsing whatchanged output; and\nb) the user sets log.abbrevCommit w/o updating his script\n\n> People using git by hand would certainly appreciate\n> --no-abbrev-commit, I suspect.\n\nThat's why I added it.\n\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1314,6 +1314,12 @@ interactive.singlekey::\n>>       linkgit:git-checkout[1]. Note that this setting is silently\n>>       ignored if portable keystroke input is not available.\n>>\n>> +log.abbrev-commit::\n>> +log.abbrevCommit::\n>> +     If true, act as if --abbrev-commit were specified on the command\n>> +     line. May be overridden with --no-abbrev-commit. Note that this setting\n>> +     is ignored by rev-list.\n>\n> Style: most of that page is written from the point of view of the\n> user\n>\n> [...]\n>\n> So maybe something like:\n>\n>        Whether to abbreviate hexadecimal commit object names in\n>        output from the 'log' family of commands.  The number of\n>        digits shown is determined by the `--abbrev` command-line\n>        option and `core.abbrev` configuration variable.  Can be\n>        overridden on the command line by --abbrev-commit /\n>        --no-abbrev-commit.  The default is false.\n>  +\n>  This does not affect the 'git diff-tree' and 'git rev-list'\n>  commands.\n\nI thought I phrased it like the nearby options in config.txt. and that\nsounds overly verbose to me, but I'll take another look.\n\n>> --- a/Documentation/git-log.txt\n>> +++ b/Documentation/git-log.txt\n>> @@ -38,6 +38,9 @@ OPTIONS\n>>       Continue listing the history of a file beyond renames\n>>       (works only for a single file).\n>>\n>> +--no-abbrev-commit::\n>> +     Don't abbreviate commit name. Useful for overridding log.abbrevCommit.\n>\n> Also useful for overriding --abbrev-commit from aliases. :)\n>\n> Shouldn't it be documented next to --abbrev-commit?\n\nAs I implemented it, --no-abbrev-commit is only honored from within\nlog.c, so I didn't want it to show up in the rev-list man page.\n\nBut, see below where I address your question about why I didn't\nimplement --no-abbrev-commit in revision.c\n\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -23,6 +23,7 @@\n>>  /* Set a default date-time format for git log (\"log.date\" config variable) */\n>>  static const char *default_date_mode = NULL;\n>>\n>> +static int default_abbrev_commit = 0;\n>>  static int default_show_root = 1;\n>>  static int decoration_style;\n>\n> Style: we try to avoid unnecessary zero initializers for variables in\n> the BSS section.\n\nOkay.\n\n> [...]\n>> @@ -89,11 +91,13 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>>                        struct rev_info *rev, struct setup_revision_opt *opt)\n>>  {\n>>       struct userformat_want w;\n>> -     int quiet = 0, source = 0;\n>> +     int quiet = 0, source = 0, no_abbrev_commit = 0;\n>>\n>>       const struct option builtin_log_options[] = {\n>> -             OPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n>> +             OPT_BOOLEAN(0, \"quiet\", &quiet, \"suppress diff output\"),\n>>               OPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n>> +             OPT_BOOLEAN(0, \"no-abbrev-commit\", &no_abbrev_commit,\n>> +                         \"don't abbreviate commit name\"),\n>\n> What happens if I do\n>\n>        git log --no-abbrev-commit --abbrev-commit\n>\n> ?  How about\n>\n>        git log --no-abbrev-commit --no-no-abbrev-commit --abbrev-commit\n>\n> ? :)  The behavior should be nicer if this is implemented in revision.c.\n\n\nIt was a thinko to put it in log.c. I added --no-abbrev-commit\nthinking of its primary use case to override log.abbrevCommit. But\nobviously it's more generally useful in revision.c (even though that\ndoesn't honor log.abbrevCommit), since it can still be used to\noverride earlier CLI options that might enable abbreviation.\n\nI'll move it. Then I can document the option next to --abbrev-commit\nwhere it makes sense.\n\n> [...]\n>> @@ -323,6 +330,11 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>>               return git_config_string(&fmt_pretty, var, value);\n>>       if (!strcmp(var, \"format.subjectprefix\"))\n>>               return git_config_string(&fmt_patch_subject_prefix, var, value);\n>> +     if (!strcasecmp(var, \"log.abbrevcommit\") ||\n>> +         !strcasecmp(var, \"log.abbrev-commit\")) {\n>\n> No need to use strcasecmp --- the vars passed to config functions\n> already have the section and variable names in lowercase.\n\nOkay, it was a cut-and-paste from ignore-errors I think.\n\n>> @@ -347,6 +359,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n>>       struct setup_revision_opt opt;\n>>\n>>       git_config(git_log_config, NULL);\n>> +     default_abbrev_commit = 0;\n>\n> Puzzling as mentioned above.\n\nWill drop.\n\n>> --- a/t/t4202-log.sh\n>> +++ b/t/t4202-log.sh\n>> @@ -450,6 +450,14 @@ test_expect_success 'log.decorate configuration' '\n>>\n>>  '\n>>\n>> +test_expect_success 'log.abbrev-commit configuration' '\n>> +     test_might_fail git config --remove-section log &&\n>> +     git log --abbrev-commit >expect &&\n>> +     git config log.abbrev-commit true &&\n>> +     git log >actual &&\n>> +     test_cmp expect actual\n>> +'\n>\n> To avoid polluting the configuration, it would be nicest to do:\n>\n>        git config log.abbrev-commit true &&\n>        test_when_finished \"git config --unset log.abbrev-commit\" &&\n>        git log >actual &&\n>\n> though it looks like some tests already protect themselves.\n\n\nOther tests in t4202 protect themselves at the start, I emulated that behavior.\n\n\n> Just because I'm curious: what happens if you do\n>\n>        git config log.abbrev-commit true\n>\n> in test_create_repo in test-lib.sh?  (I.e., are there many tests that\n> would be confused by this?)  Tests tend to be more picky than user\n> scripts about the output of git but it might still be an ok way to\n> get a vague sense of the impact.\n\nI'll let you know. :-)\n\n> Hope that helps, and thanks for a pleasant read.\n\nThanks for the quick review.\n\nj.\n"},{"id":"167863","messageId":"20110514201921.GA10758@elie","threadId":"27352","inReplyTo":"BANLkTi=MSfRhUhY1jkRC0agQNp7WHDG9FQ@mail.gmail.com","subject":"[PATCH] add, merge, diff: do not use strcasecmp to compare config variable names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-14T20:19:21Z","receivedAt":"2011-05-14T20:19:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The config machinery already makes section and variable names\nlowercase when parsing them, so using strcasecmp for comparison just\nfeels wasteful.  No noticeable change intended.\n\nNoticed-by: Jay Soffian <jaysoffian@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nJay Soffian wrote:\n> On Sat, May 14, 2011 at 3:01 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> No need to use strcasecmp --- the vars passed to config functions\n>> already have the section and variable names in lowercase.\n>\n> Okay, it was a cut-and-paste from ignore-errors I think.\n\nGood catch.\n\n builtin/add.c     |    4 ++--\n merge-recursive.c |    6 +++---\n xdiff-interface.c |    2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 704141f..e57abdd 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -331,8 +331,8 @@ static struct option builtin_add_options[] = {\n \n static int add_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcasecmp(var, \"add.ignoreerrors\") ||\n-\t    !strcasecmp(var, \"add.ignore-errors\")) {\n+\tif (!strcmp(var, \"add.ignoreerrors\") ||\n+\t    !strcmp(var, \"add.ignore-errors\")) {\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex ecb1806..07ad1a3 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1721,15 +1721,15 @@ int merge_recursive_generic(struct merge_options *o,\n static int merge_recursive_config(const char *var, const char *value, void *cb)\n {\n \tstruct merge_options *o = cb;\n-\tif (!strcasecmp(var, \"merge.verbosity\")) {\n+\tif (!strcmp(var, \"merge.verbosity\")) {\n \t\to->verbosity = git_config_int(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcasecmp(var, \"diff.renamelimit\")) {\n+\tif (!strcmp(var, \"diff.renamelimit\")) {\n \t\to->diff_rename_limit = git_config_int(var, value);\n \t\treturn 0;\n \t}\n-\tif (!strcasecmp(var, \"merge.renamelimit\")) {\n+\tif (!strcmp(var, \"merge.renamelimit\")) {\n \t\to->merge_rename_limit = git_config_int(var, value);\n \t\treturn 0;\n \t}\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex 164581f..0e2c169 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -347,7 +347,7 @@ int git_xmerge_style = -1;\n \n int git_xmerge_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcasecmp(var, \"merge.conflictstyle\")) {\n+\tif (!strcmp(var, \"merge.conflictstyle\")) {\n \t\tif (!value)\n \t\t\tdie(\"'%s' is not a boolean\", var);\n \t\tif (!strcmp(value, \"diff3\"))\n-- \n1.7.5.1\n"},{"id":"167864","messageId":"1305406038-36435-1-git-send-email-jaysoffian@gmail.com","threadId":"27352","inReplyTo":"20110514190122.GA16851@elie","subject":"[PATCH v2] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T20:47:18Z","receivedAt":"2011-05-14T20:47:18Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Add log.abbrevCommit config variable as a convenience for users who\noften use --abbrev-commit with git log and friends. Allow the option\nto be overridden with --no-abbrev-commit.\n\n(Also, a drive-by spelling correction in git log's short help.)\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nAddresses Jonathan's feedback.\n\n> Just because I'm curious: what happens if you do\n> \n>        git config log.abbrev-commit true\n> \n> in test_create_repo in test-lib.sh?  (I.e., are there many tests that\n> would be confused by this?)  Tests tend to be more picky than user\n> scripts about the output of git but it might still be an ok way to\n> get a vague sense of the impact.\n\nAs expected, it breaks all the tests which rely on the output of show,\nwhatchanged, and log not having abbreviated commit names. They could\nall be adjusted to pass '--no-abbrev-commit', but I think that's\npointless churn.\n\n Documentation/config.txt         |    5 +++++\n Documentation/pretty-options.txt |    5 +++++\n builtin/log.c                    |    8 +++++++-\n revision.c                       |    2 ++\n t/t4202-log.sh                   |   12 ++++++++++++\n 5 files changed, 31 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 285c7f73ca..8a3a1d08f7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1314,6 +1314,11 @@ interactive.singlekey::\n \tlinkgit:git-checkout[1]. Note that this setting is silently\n \tignored if portable keystroke input is not available.\n \n+log.abbrevCommit::\n+\tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n+\tlinkgit:git-whatchanged[1] assume `\\--abbrev-commit`. You may\n+\toverride this option with `\\--no-abbrev-commit`.\n+\n log.date::\n \tSet the default date-time mode for the 'log' command.\n \tSetting a value for log.date is similar to using 'git log''s\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex d5c977262a..2a3dc8664f 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -19,6 +19,11 @@ configuration (see linkgit:git-config[1]).\n This should make \"--pretty=oneline\" a whole lot more readable for\n people using 80-column terminals.\n \n+--no-abbrev-commit::\n+\tShow the full 40-byte hexadecimal commit object name. This negates\n+\t`--abbrev-commit` and those options which imply it such as\n+\t\"--oneline\". It also overrides the 'log.abbrevCommit' variable.\n+\n --oneline::\n \tThis is a shorthand for \"--pretty=oneline --abbrev-commit\"\n \tused together.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f6219909a7..540d1a473a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -23,6 +23,7 @@\n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n \n+static int default_abbrev_commit;\n static int default_show_root = 1;\n static int decoration_style;\n static int decoration_given;\n@@ -77,6 +78,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \t\tget_commit_format(fmt_pretty, rev);\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n+\trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n@@ -92,7 +94,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tint quiet = 0, source = 0;\n \n \tconst struct option builtin_log_options[] = {\n-\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n+\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"suppress diff output\"),\n \t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n \t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n \t\t  PARSE_OPT_OPTARG, decorate_callback},\n@@ -323,6 +325,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcmp(var, \"log.abbrevcommit\")) {\n+\t\tdefault_abbrev_commit = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"log.date\"))\n \t\treturn git_config_string(&default_date_mode, var, value);\n \tif (!strcmp(var, \"log.decorate\")) {\ndiff --git a/revision.c b/revision.c\nindex a7cf79bf2e..86028917e1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1429,6 +1429,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\trevs->abbrev = 40;\n \t} else if (!strcmp(arg, \"--abbrev-commit\")) {\n \t\trevs->abbrev_commit = 1;\n+\t} else if (!strcmp(arg, \"--no-abbrev-commit\")) {\n+\t\trevs->abbrev_commit = 0;\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 2fcc31a6f3..a53a86a989 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -450,6 +450,18 @@ test_expect_success 'log.decorate configuration' '\n \n '\n \n+test_expect_success 'log.abbrev-commit configuration' '\n+\ttest_might_fail git config --remove-section log &&\n+\ttest_when_finished \"git config --unset log.abbrevCommit\" &&\n+\tgit log >expect.full &&\n+\tgit log --abbrev-commit >expect.abbrev &&\n+\tgit config log.abbrevCommit true &&\n+\tgit log >actual.abbrev &&\n+\ttest_cmp expect.abbrev actual.abbrev &&\n+\tgit log --no-abbrev-commit >actual.full &&\n+\ttest_cmp expect.full actual.full\n+'\n+\n test_expect_success 'show added path under \"--follow -M\"' '\n \t# This tests for a regression introduced in v1.7.2-rc0~103^2~2\n \ttest_create_repo regression &&\n-- \n1.7.5.1.290.g2f216.dirty\n"},{"id":"167866","messageId":"20110514215543.GA23648@elie","threadId":"27352","inReplyTo":"1305406038-36435-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] Add log.abbrevCommit config variable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-14T21:55:43Z","receivedAt":"2011-05-14T21:55:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jay Soffian wrote:\n\n> Add log.abbrevCommit config variable as a convenience for users who\n> often use --abbrev-commit with git log and friends. Allow the option\n> to be overridden with --no-abbrev-commit.\n\nBased on a quick google code search, this will break scripts. :/\nFor example, tig uses \"git log --pretty=raw\" to get revision lists,\nincluding expecting 40-digit commit ids.\n\nBut aside from that, this looks good to me.  I'm not sure what to\nsuggest: go out and spread the word about diff-tree? suppress this\nconfiguration when --pretty or --format is passed?  There's no obvious\ngood answer.\n"},{"id":"167868","messageId":"BANLkTi=og+e6JW8t+sCvkFJUnR=SrFtWGg@mail.gmail.com","threadId":"27352","inReplyTo":"20110514215543.GA23648@elie","subject":"Re: [PATCH v2] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T22:22:38Z","receivedAt":"2011-05-14T22:22:38Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, May 14, 2011 at 5:55 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Jay Soffian wrote:\n>\n>> Add log.abbrevCommit config variable as a convenience for users who\n>> often use --abbrev-commit with git log and friends. Allow the option\n>> to be overridden with --no-abbrev-commit.\n>\n> Based on a quick google code search, this will break scripts. :/\n> For example, tig uses \"git log --pretty=raw\" to get revision lists,\n> including expecting 40-digit commit ids.\n\nHmfph.\n\n> But aside from that, this looks good to me.  I'm not sure what to\n> suggest: go out and spread the word about diff-tree? suppress this\n> configuration when --pretty or --format is passed?  There's no obvious\n> good answer.\n\nAh, see:\n\n- 635530a2fc (log --pretty/--oneline: ignore log.decorate, 2010-04-06)\n- 4f62c2bc57 (log.decorate: only ignore it under \"log --pretty=raw\", 2010-04-08)\n\nSo I'll do the same thing for log.abbrevCommit.\n\nj.\n"},{"id":"167870","messageId":"1305413341-56450-1-git-send-email-jaysoffian@gmail.com","threadId":"27352","inReplyTo":"BANLkTi=og+e6JW8t+sCvkFJUnR=SrFtWGg@mail.gmail.com","subject":"[PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-14T22:49:01Z","receivedAt":"2011-05-14T22:49:01Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Add log.abbrevCommit config variable as a convenience for users who\noften use --abbrev-commit with git log and friends. Allow the option\nto be overridden with --no-abbrev-commit.\n\n(Also, a drive-by spelling correction in git log's short help.)\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nChanges from v2: ignore log.abbrevCommit when given --pretty=raw\n\n Documentation/config.txt         |    5 +++++\n Documentation/pretty-options.txt |    5 +++++\n builtin/log.c                    |   14 +++++++++++++-\n revision.c                       |    3 +++\n revision.h                       |    1 +\n t/t4202-log.sh                   |   15 +++++++++++++++\n 6 files changed, 42 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 285c7f73ca..8a3a1d08f7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1314,6 +1314,11 @@ interactive.singlekey::\n \tlinkgit:git-checkout[1]. Note that this setting is silently\n \tignored if portable keystroke input is not available.\n \n+log.abbrevCommit::\n+\tIf true, makes linkgit:git-log[1], linkgit:git-show[1], and\n+\tlinkgit:git-whatchanged[1] assume `\\--abbrev-commit`. You may\n+\toverride this option with `\\--no-abbrev-commit`.\n+\n log.date::\n \tSet the default date-time mode for the 'log' command.\n \tSetting a value for log.date is similar to using 'git log''s\ndiff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt\nindex d5c977262a..2a3dc8664f 100644\n--- a/Documentation/pretty-options.txt\n+++ b/Documentation/pretty-options.txt\n@@ -19,6 +19,11 @@ configuration (see linkgit:git-config[1]).\n This should make \"--pretty=oneline\" a whole lot more readable for\n people using 80-column terminals.\n \n+--no-abbrev-commit::\n+\tShow the full 40-byte hexadecimal commit object name. This negates\n+\t`--abbrev-commit` and those options which imply it such as\n+\t\"--oneline\". It also overrides the 'log.abbrevCommit' variable.\n+\n --oneline::\n \tThis is a shorthand for \"--pretty=oneline --abbrev-commit\"\n \tused together.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f6219909a7..460fc15d8f 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -23,6 +23,7 @@\n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n \n+static int default_abbrev_commit;\n static int default_show_root = 1;\n static int decoration_style;\n static int decoration_given;\n@@ -77,6 +78,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n \t\tget_commit_format(fmt_pretty, rev);\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n+\trev->abbrev_commit = default_abbrev_commit;\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n \tDIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);\n@@ -92,7 +94,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \tint quiet = 0, source = 0;\n \n \tconst struct option builtin_log_options[] = {\n-\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"supress diff output\"),\n+\t\tOPT_BOOLEAN(0, \"quiet\", &quiet, \"suppress diff output\"),\n \t\tOPT_BOOLEAN(0, \"source\", &source, \"show source\"),\n \t\t{ OPTION_CALLBACK, 0, \"decorate\", NULL, NULL, \"decorate options\",\n \t\t  PARSE_OPT_OPTARG, decorate_callback},\n@@ -137,6 +139,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t    && rev->commit_format == CMIT_FMT_RAW)\n \t\tdecoration_style = 0;\n \n+\t/* ditto for log.abbrevCommit */\n+\tif (!rev->abbrev_commit_given && rev->abbrev_commit\n+\t    && rev->commit_format == CMIT_FMT_RAW)\n+\t\trev->abbrev_commit = 0;\n+\n+\n \tif (decoration_style) {\n \t\trev->show_decorations = 1;\n \t\tload_ref_decorations(decoration_style);\n@@ -323,6 +331,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_pretty, var, value);\n \tif (!strcmp(var, \"format.subjectprefix\"))\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n+\tif (!strcmp(var, \"log.abbrevcommit\")) {\n+\t\tdefault_abbrev_commit = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"log.date\"))\n \t\treturn git_config_string(&default_date_mode, var, value);\n \tif (!strcmp(var, \"log.decorate\")) {\ndiff --git a/revision.c b/revision.c\nindex a7cf79bf2e..be74bf92f5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1429,6 +1429,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\trevs->abbrev = 40;\n \t} else if (!strcmp(arg, \"--abbrev-commit\")) {\n \t\trevs->abbrev_commit = 1;\n+\t\trevs->abbrev_commit_given = 1;\n+\t} else if (!strcmp(arg, \"--no-abbrev-commit\")) {\n+\t\trevs->abbrev_commit = 0;\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\ndiff --git a/revision.h b/revision.h\nindex bca9947977..5e1f5c7e8f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -90,6 +90,7 @@ struct rev_info {\n \t\t\tshow_notes_given:1,\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n+\t\t\tabbrev_commit_given:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n \t\t\tdate_mode_explicit:1;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 2fcc31a6f3..9e8afd65a2 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -450,6 +450,21 @@ test_expect_success 'log.decorate configuration' '\n \n '\n \n+test_expect_success 'log.abbrev-commit configuration' '\n+\ttest_might_fail git config --remove-section log &&\n+\ttest_when_finished \"git config --unset log.abbrevCommit\" &&\n+\tgit log >expect.full &&\n+\tgit log --pretty=raw >expect.raw &&\n+\tgit log --abbrev-commit >expect.abbrev &&\n+\tgit config log.abbrevCommit true &&\n+\tgit log >actual.abbrev &&\n+\ttest_cmp expect.abbrev actual.abbrev &&\n+\tgit log --no-abbrev-commit >actual.full &&\n+\ttest_cmp expect.full actual.full &&\n+\tgit log --pretty=raw >actual.raw &&\n+\ttest_cmp expect.raw actual.raw\n+'\n+\n test_expect_success 'show added path under \"--follow -M\"' '\n \t# This tests for a regression introduced in v1.7.2-rc0~103^2~2\n \ttest_create_repo regression &&\n-- \n1.7.5.1.290.g992321\n"},{"id":"167873","messageId":"7vliy8rawv.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"20110514190122.GA16851@elie","subject":"Re: [PATCH] Add log.abbrev-commit config option","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2011-05-15T01:48:48Z","receivedAt":"2011-05-15T01:48:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hm, that wouldn't have been my hunch.  Are you aware of any scripts\n> that parse \"git whatchanged\" output?\n>\n> More worrying is \"git log --format=raw\".\n\nBoth are very sane worries. I am not interested at all in queuing this\nchange without any careful migration consideration.\n\nThanks.\n"},{"id":"167872","messageId":"7vhb8wrap2.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"20110514201921.GA10758@elie","subject":"Re: [PATCH] add, merge, diff: do not use strcasecmp to compare config variable names","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2011-05-15T01:53:29Z","receivedAt":"2011-05-15T01:53:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> The config machinery already makes section and variable names\n> lowercase when parsing them, so using strcasecmp for comparison just\n> feels wasteful.  No noticeable change intended.\n\nA quick \"grep 'strcasecmp(va*r*, \"[^\"]*\\.' yields exactly the ones covered\nby this patch.\n\nThanks, I've been meaning to change these while reading merge-recursive.\n"},{"id":"167894","messageId":"BANLkTimm6Z8bRyM=_=mDRp2CZ6pypP54AA@mail.gmail.com","threadId":"27352","inReplyTo":"1305413341-56450-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-15T13:25:13Z","receivedAt":"2011-05-15T13:25:13Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, May 14, 2011 at 6:49 PM, Jay Soffian <jaysoffian@gmail.com> wrote:\n> Add log.abbrevCommit config variable as a convenience for users who\n> often use --abbrev-commit with git log and friends. Allow the option\n> to be overridden with --no-abbrev-commit.\n>\n> (Also, a drive-by spelling correction in git log's short help.)\n>\n> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>\n> ---\n> Changes from v2: ignore log.abbrevCommit when given --pretty=raw\n\nWhich I meant to mention in the commit message. Updated commit message:\n\n---\n\nAdd log.abbrevCommit config variable\n\nAdd log.abbrevCommit config variable as a convenience for users who\noften use --abbrev-commit with git log and friends. Allow the option\nto be overridden with --no-abbrev-commit. Per 635530a2fc and 4f62c2bc57,\nthe config variable is ignored when log is given \"--pretty=raw\".\n\n(Also, a drive-by spelling correction in git log's short help.)\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n"},{"id":"167939","messageId":"7vei3zoab9.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"1305413341-56450-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T22:42:18Z","receivedAt":"2011-05-15T22:42:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> @@ -137,6 +139,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \t    && rev->commit_format == CMIT_FMT_RAW)\n>  \t\tdecoration_style = 0;\n>  \n> +\t/* ditto for log.abbrevCommit */\n> +\tif (!rev->abbrev_commit_given && rev->abbrev_commit\n> +\t    && rev->commit_format == CMIT_FMT_RAW)\n> +\t\trev->abbrev_commit = 0;\n> +\n> +\n\nI am not entirely happy about this change. The \"ditto\" refers to an ugly\nworkaround we had to add with 4f62c2b (log.decorate: only ignore it under\n\"log --pretty=raw\", 2010-04-08) only because it was too late to revert the\nchange in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was\nabout to become part of 1.7.2-rc0 release. If we knew better, we probably\nwouldn't have added the log.decorate variable that requires this hack that\nrequires us to say that 'log --pretty=raw' is special.\n\nIf we stop before adding a new configuration, we do not have to repeat the\nsame mistake we made earlier.\n\nI dunno.\n"},{"id":"167962","messageId":"BANLkTi=iFgJ12=_amoxT8x+hNMEhQtOVBg@mail.gmail.com","threadId":"27352","inReplyTo":"7vei3zoab9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-16T05:53:52Z","receivedAt":"2011-05-16T05:53:52Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, May 15, 2011 at 6:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> I am not entirely happy about this change. The \"ditto\" refers to an ugly\n> workaround we had to add with 4f62c2b (log.decorate: only ignore it under\n> \"log --pretty=raw\", 2010-04-08) only because it was too late to revert the\n> change in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was\n> about to become part of 1.7.2-rc0 release. If we knew better, we probably\n> wouldn't have added the log.decorate variable that requires this hack that\n> requires us to say that 'log --pretty=raw' is special.\n>\n> If we stop before adding a new configuration, we do not have to repeat the\n> same mistake we made earlier.\n>\n> I dunno.\n\nI disagree that log.decorate is a mistake and that the workaround is\nugly. It would be nice if we didn't need the workaround, but I don't\nthink we should have to tie our hands because there are scripts that\nare parsing git log's output. I think it's a pragmatic solution to\nremaining backwards compatible while still being able to provide new\nfunctionality.\n\nFWIW, I use log.decorate, and I'd like to be able to use log.abbrevCommit.\n\nj.\n"},{"id":"167971","messageId":"20110516070047.GA26270@elie","threadId":"27352","inReplyTo":"BANLkTi=iFgJ12=_amoxT8x+hNMEhQtOVBg@mail.gmail.com","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-16T07:00:47Z","receivedAt":"2011-05-16T07:00:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jay Soffian wrote:\n> On Sun, May 15, 2011 at 6:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n>> I am not entirely happy about this change. The \"ditto\" refers to an ugly\n>> workaround we had to add with 4f62c2b (log.decorate: only ignore it under\n>> \"log --pretty=raw\", 2010-04-08) only because it was too late to revert the\n>> change in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was\n>> about to become part of 1.7.2-rc0 release. If we knew better, we probably\n>> wouldn't have added the log.decorate variable that requires this hack that\n>> requires us to say that 'log --pretty=raw' is special.\n>>\n>> If we stop before adding a new configuration, we do not have to repeat the\n>> same mistake we made earlier.\n>>\n>> I dunno.\n>\n> I disagree that log.decorate is a mistake and that the workaround is\n> ugly.\n\nI suppose part of what Junio is saying is that by the time the commits\nreferenced above were written, git had already broken some scripts\n(including gitk) and those changes were part of a desparate attempt to\ncontain the damage.  So they are not a great example to look to for\nthe sort of smooth transition it is possible to set up proactively.\n\nFor example, maybe (after fixing the scripts we already know about,\nsuch as tig) we could add the log.abbrevcommit variable right away but\nadvertise it as experimental:\n\n\t*Warning* This option is experimental and will break your\n\tscripts.  It is only provided to give script authors a\n\tchance to test this functionality and fix their scripts\n\tbefore the feature is advertised in earnest.\n\nOne transition plan could look like this:\n\n 1. In the release notes to v1.7.6, mention that there is a change\n    on the horizon that would break people's scripts and encourage\n    script authors to switch to \"rev-list | diff-tree -s --stdin\"\n    if their scripts depend on the details of \"git log\" format\n    (in particular, if their scripts do not work correctly after\n    s/log/log --abbrev-commit/).  Introduce the log.abbrevcommit\n    variable to help people test, guarded by a compile-time\n    option and disabled by default.\n\n 2. In v1.7.7, introduce the log.abbrevcommit variable, advertised\n    as \"This will break your system --- don't use it unless you\n    are trying to find such breakage and fix it\".\n\n 3. In v1.8.0, introduce the variable in earnest and recommend\n    that people use it.\n\nI think step 1 is going too far --- it should be possible to give\nusers rope like this without worrying that they are going to be\nirresponsible about it.\n\nNow, returning to \"log --pretty=raw\".  Is it plumbing or not?  It\nwould be nice to advertise whichever way it is decided (I guess it is\nde facto plumbing) in the \"git log\" reference documentation and to\nfollow that decision in cases like this one.\n\nThanks for some food for thought.\n\nCiao,\nJonathan\n"},{"id":"167972","messageId":"BANLkTimCUU=CNiVnka79n=OKki0wQQtQCA@mail.gmail.com","threadId":"27352","inReplyTo":"20110516070047.GA26270@elie","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-16T07:18:21Z","receivedAt":"2011-05-16T07:18:21Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, May 16, 2011 at 3:00 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> I suppose part of what Junio is saying is that by the time the commits\n> referenced above were written, git had already broken some scripts\n> (including gitk) and those changes were part of a desparate attempt to\n> contain the damage.  So they are not a great example to look to for\n> the sort of smooth transition it is possible to set up proactively.\n\nWell, then why wasn't the change simply reverted?\n\n> For example, maybe (after fixing the scripts we already know about,\n> such as tig) we could add the log.abbrevcommit variable right away but\n> advertise it as experimental:\n>\n>        *Warning* This option is experimental and will break your\n>        scripts.  It is only provided to give script authors a\n>        chance to test this functionality and fix their scripts\n>        before the feature is advertised in earnest.\n>\n> One transition plan could look like this:\n>\n>  1. In the release notes to v1.7.6, mention that there is a change\n>    on the horizon that would break people's scripts and encourage\n>    script authors to switch to \"rev-list | diff-tree -s --stdin\"\n>    if their scripts depend on the details of \"git log\" format\n>    (in particular, if their scripts do not work correctly after\n>    s/log/log --abbrev-commit/).  Introduce the log.abbrevcommit\n>    variable to help people test, guarded by a compile-time\n>    option and disabled by default.\n>\n>  2. In v1.7.7, introduce the log.abbrevcommit variable, advertised\n>    as \"This will break your system --- don't use it unless you\n>    are trying to find such breakage and fix it\".\n>\n>  3. In v1.8.0, introduce the variable in earnest and recommend\n>    that people use it.\n>\n> I think step 1 is going too far --- it should be possible to give\n> users rope like this without worrying that they are going to be\n> irresponsible about it.\n>\n> Now, returning to \"log --pretty=raw\".  Is it plumbing or not?  It\n> would be nice to advertise whichever way it is decided (I guess it is\n> de facto plumbing) in the \"git log\" reference documentation and to\n> follow that decision in cases like this one.\n\nTo my mind, we call \"--pretty=raw\" de-facto plumbing and keep the\nchange as is. Though perhaps, there is a cleaner implementation if we\nsay that --pretty=raw is indeed plumbing.\n\n> Thanks for some food for thought.\n\nAnd here I thought this was a simple one. :-(\n\nj.\n"},{"id":"167973","messageId":"4DD0DF2E.5070901@drmicha.warpmail.net","threadId":"27352","inReplyTo":"7vliy8rawv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add log.abbrev-commit config option","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-16T08:24:14Z","receivedAt":"2011-05-16T08:24:14Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 15.05.2011 03:48:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n> \n>> Hm, that wouldn't have been my hunch.  Are you aware of any scripts\n>> that parse \"git whatchanged\" output?\n>>\n>> More worrying is \"git log --format=raw\".\n> \n> Both are very sane worries. I am not interested at all in queuing this\n> change without any careful migration consideration.\n> \n> Thanks.\n\nAnother instance of \"config for option\". Note that, again, it has the\nsame potential problems as a general approach...\n\nMichael\n"},{"id":"168072","messageId":"7vei3xgsy3.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"20110516070047.GA26270@elie","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-17T17:03:48Z","receivedAt":"2011-05-17T17:03:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Jay Soffian wrote:\n>> On Sun, May 15, 2011 at 6:42 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>>> I am not entirely happy about this change. The \"ditto\" refers to an ugly\n>>> workaround we had to add with 4f62c2b (log.decorate: only ignore it under\n>>> \"log --pretty=raw\", 2010-04-08) only because it was too late to revert the\n>>> change in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was\n>>> about to become part of 1.7.2-rc0 release. If we knew better, we probably\n>>> wouldn't have added the log.decorate variable that requires this hack that\n>>> requires us to say that 'log --pretty=raw' is special.\n>>>\n>>> If we stop before adding a new configuration, we do not have to repeat the\n>>> same mistake we made earlier.\n>>>\n>>> I dunno.\n>>\n>> I disagree that log.decorate is a mistake and that the workaround is\n>> ugly.\n>\n> I suppose part of what Junio is saying is that by the time the commits\n> referenced above were written, git had already broken some scripts\n> (including gitk) and those changes were part of a desparate attempt to\n> contain the damage.  So they are not a great example to look to for\n> the sort of smooth transition it is possible to set up proactively.\n\nThanks for rephasing.\n\n> One transition plan could look like this:\n> ...\n> Now, returning to \"log --pretty=raw\".  Is it plumbing or not?  It\n> would be nice to advertise whichever way it is decided (I guess it is\n> de facto plumbing) in the \"git log\" reference documentation and to\n> follow that decision in cases like this one.\n>\n> Thanks for some food for thought.\n\nAlthough I find the cop-out \"if you set this variable, some third-party\ntools may break---please do not complain to us, but send bug reports to\nthese third-party folks\" somewhat attractive, I am reluctant to take that\nposition. I suspect some projects do work that way, and if it works for\nthem, it may turn out that we wouldn't get much flak doing so ourselves,\nbut still...\n\nBut having slept on this, I would agree that it is a pragmatic solution to\ndeclare \"pretty=raw _is_ special\" and keep it that way.  Let's queue v3\nfrom Jay and see what happens.\n\nThanks.\n"},{"id":"168076","messageId":"7vy625f9fc.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"1305413341-56450-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-17T18:50:47Z","receivedAt":"2011-05-17T18:50:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian <jaysoffian@gmail.com> writes:\n\n> @@ -137,6 +139,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \t    && rev->commit_format == CMIT_FMT_RAW)\n>  \t\tdecoration_style = 0;\n>  \n> +\t/* ditto for log.abbrevCommit */\n> +\tif (!rev->abbrev_commit_given && rev->abbrev_commit\n> +\t    && rev->commit_format == CMIT_FMT_RAW)\n> +\t\trev->abbrev_commit = 0;\n> +\n> +\n\nThis is not exactly \"ditto\"; the lines before this hunk look like this:\n\n\t/*\n\t * defeat log.decorate configuration interacting with --pretty=raw\n\t * from the command line.\n\t */\n\tif (!decoration_given && rev->pretty_given\n\t    && rev->commit_format == CMIT_FMT_RAW)\n\t\tdecoration_style = 0;\n\nThe check for pretty-given and commit-format being raw is to catch only\nthe case where the command line said --pretty=raw, excluding any case the\ncommit-format is set to raw without any explicit --pretty=raw on the\ncommand line.  So at the logical level, this one should be read as if it\nwas written like this:\n\n\tif (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {\n        \t/* \"log --pretty=raw\" is special */\n        \tif (!decoration_given)\n\t\t\tdecoration_style = 0;\n\t}\n\neven though the existing one makes it look as if checking decoration-given\nand pretty-given together when commit-format is set to raw.\n\nCare to explain why your check has to be different?  If there is no reason\nto use a different logic, then a natural thing to do is to rewrite the\nexisting decoration logic and add yours like this:\n\n\tif (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {\n        \t/* \"log --pretty=raw\" is special */\n        \tif (!decoration_given)\n\t\t\tdecoration_style = 0;\n\t\tif (!abbrev_commit_given)\n\t\t\trev->abbrev_commit = 0;\n\t}\n"},{"id":"168077","messageId":"BANLkTikW2Y0iAW6jiVMN9ifvTezY+iO+bg@mail.gmail.com","threadId":"27352","inReplyTo":"7vy625f9fc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-17T19:08:03Z","receivedAt":"2011-05-17T19:08:03Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, May 17, 2011 at 2:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Care to explain why your check has to be different?\n\nIt doesn't. I thought through it poorly.\n\n> If there is no reason\n> to use a different logic, then a natural thing to do is to rewrite the\n> existing decoration logic and add yours like this:\n>\n>        if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {\n>                /* \"log --pretty=raw\" is special */\n>                if (!decoration_given)\n>                        decoration_style = 0;\n>                if (!abbrev_commit_given)\n>                        rev->abbrev_commit = 0;\n>        }\n\nIt would be lovely if you could squash that in.\n\nThanks,\n\nj.\n"},{"id":"168084","messageId":"7vtyctf142.fsf@alter.siamese.dyndns.org","threadId":"27352","inReplyTo":"7vei3xgsy3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-17T21:50:21Z","receivedAt":"2011-05-17T21:50:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> But having slept on this, I would agree that it is a pragmatic solution to\n> declare \"pretty=raw _is_ special\" and keep it that way.  Let's queue v3\n> from Jay and see what happens.\n\nHaving looked at this a bit more, there seems to be a special case in\ncmd_log_reflog() that the command wants to default to abbrevCommit.\n\nI haven't applied v3 (nor my fix-up in the discussion) yet, as this is not\nmy itch, but have you tested all commands from \"log\" family with your\npatch, including \"git reflog\"?\n"},{"id":"168090","messageId":"BANLkTinaP8G3Kt0bMZ6QEW6SFNau7dRLMA@mail.gmail.com","threadId":"27352","inReplyTo":"7vtyctf142.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Add log.abbrevCommit config variable","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-05-18T01:05:23Z","receivedAt":"2011-05-18T01:05:23Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, May 17, 2011 at 5:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> I haven't applied v3 (nor my fix-up in the discussion) yet, as this is not\n> my itch, but have you tested all commands from \"log\" family with your\n> patch, including \"git reflog\"?\n\nGah. Thank you for catching that, I had checked it (I think) with v1.\nI should've added a test then, which I've done for v4. Sending\nshortly.\n\nj.\n"}]}