{"thread":{"id":"22683","subject":"[PATCH] Add `log.decorate' configuration variable.","startedAt":"2010-02-16T23:39:52Z","lastAt":"2010-02-17T18:41:43Z","messageCount":9,"participants":["Steven Drake","Junio C Hamano","Bert Wesarg","Heiko Voigt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"134810","messageId":"alpine.LNX.2.00.1002171239430.2477@vqena.qenxr.bet.am","threadId":"22683","inReplyTo":null,"subject":"[PATCH] Add `log.decorate' configuration variable.","fromName":"Steven Drake","fromEmail":"sdrake@xnet.co.nz","sentAt":"2010-02-16T23:39:52Z","receivedAt":"2010-02-16T23:39:52Z","isPatch":true,"sender":{"key":"sdrake@xnet.co.nz","avatar":null},"body":"This alows the 'git-log --decorate' to be enabled by default so that normal\nlog outout contains ant ref names of commits that are shown.\n\nSigned-off-by: Steven Drake <sdrake@xnet.co.nz>\n---\n Documentation/config.txt |    7 +++++++\n builtin-log.c            |    9 ++++++++-\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1aead58..8359eb5 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1217,6 +1217,13 @@ log.date::\n \tfollowing alternatives: {relative,local,default,iso,rfc,short}.\n \tSee linkgit:git-log[1].\n \n+log.decorate::\n+\tPrint out the ref names of any commits that are shown by the log\n+\tcommand. If 'short' is specified, the ref name prefixes 'refs/heads/',\n+\t'refs/tags/' and 'refs/remotes/' will not be printed. If 'full' is\n+\tspecified, the full ref name (including prefix) will be printed.\n+\tThis is the same as the log commands '--decorate' option.\n+\n log.showroot::\n \tIf true, the initial commit will be shown as a big creation event.\n \tThis is equivalent to a diff against an empty tree.\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 89f8d60..cd6158c 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -24,6 +24,7 @@\n static const char *default_date_mode = NULL;\n \n static int default_show_root = 1;\n+static int decoration_style = 0;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n@@ -35,7 +36,6 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\t      struct rev_info *rev)\n {\n \tint i;\n-\tint decoration_style = 0;\n \n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n@@ -249,6 +249,13 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n \tif (!strcmp(var, \"log.date\"))\n \t\treturn git_config_string(&default_date_mode, var, value);\n+\tif (!strcmp(var, \"log.decorate\")) {\n+\t\tif (!strcmp(value, \"full\"))\n+\t\t\tdecoration_style = DECORATE_FULL_REFS;\n+\t\telse if (!strcmp(value, \"short\"))\n+\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"log.showroot\")) {\n \t\tdefault_show_root = git_config_bool(var, value);\n \t\treturn 0;\n-- \n1.6.6\n"},{"id":"134825","messageId":"7vljespt2l.fsf@alter.siamese.dyndns.org","threadId":"22683","inReplyTo":"alpine.LNX.2.00.1002171239430.2477@vqena.qenxr.bet.am","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-17T01:08:34Z","receivedAt":"2010-02-17T01:08:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Drake <sdrake@xnet.co.nz> writes:\n\n> This alows the 'git-log --decorate' to be enabled by default so that normal\n> log outout contains ant ref names of commits that are shown.\n>\n> Signed-off-by: Steven Drake <sdrake@xnet.co.nz>\n> ---\n\nThanks.\n\nThis needs some test to make sure that it triggers when configuration is\nset, it doesn't when configuration is not set, and it doesn't for commands\nin log family when it shouldn't (most notably, format-patch).\n\n> +log.decorate::\n> +\tPrint out the ref names of any commits that are shown by the log\n> +\tcommand. If 'short' is specified, the ref name prefixes 'refs/heads/',\n> +\t'refs/tags/' and 'refs/remotes/' will not be printed. If 'full' is\n> +\tspecified, the full ref name (including prefix) will be printed.\n> +\tThis is the same as the log commands '--decorate' option.\n\nThis should be the same as --decorate option, so it should be possible to\nset it as a boolean true to mean \"short\", i.e.\n\n\t[log]\n        \tdecorate\n\t\tdecorate = true\n\nshould be treated exactly the same way as\n\n\t[log]\n        \tdecorate = short\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 89f8d60..cd6158c 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -249,6 +249,13 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>  \t\treturn git_config_string(&fmt_patch_subject_prefix, var, value);\n>  \tif (!strcmp(var, \"log.date\"))\n>  \t\treturn git_config_string(&default_date_mode, var, value);\n> +\tif (!strcmp(var, \"log.decorate\")) {\n> +\t\tif (!strcmp(value, \"full\"))\n> +\t\t\tdecoration_style = DECORATE_FULL_REFS;\n> +\t\telse if (!strcmp(value, \"short\"))\n> +\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n> +\t\treturn 0;\n\nHence you need to be prepared to see (value == NULL) here without\nsegfaulting.  Perhaps something like this patch on top of yours.\n\n cache.h       |    1 +\n config.c      |   12 +++++++++---\n builtin-log.c |   11 +++++++++++\n 3 files changed, 21 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d478eff..24addea 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -923,6 +923,7 @@ extern int git_parse_ulong(const char *, unsigned long *);\n extern int git_config_int(const char *, const char *);\n extern unsigned long git_config_ulong(const char *, const char *);\n extern int git_config_bool_or_int(const char *, const char *, int *);\n+extern int git_config_maybe_bool(const char *, const char *);\n extern int git_config_bool(const char *, const char *);\n extern int git_config_string(const char **, const char *, const char *);\n extern int git_config_pathname(const char **, const char *, const char *);\ndiff --git a/config.c b/config.c\nindex 6963fbe..6642d30 100644\n--- a/config.c\n+++ b/config.c\n@@ -322,9 +322,8 @@ unsigned long git_config_ulong(const char *name, const char *value)\n \treturn ret;\n }\n \n-int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n+int git_config_maybe_bool(const char *name, const char *value)\n {\n-\t*is_bool = 1;\n \tif (!value)\n \t\treturn 1;\n \tif (!*value)\n@@ -333,7 +332,14 @@ int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n \t\treturn 1;\n \tif (!strcasecmp(value, \"false\") || !strcasecmp(value, \"no\") || !strcasecmp(value, \"off\"))\n \t\treturn 0;\n-\t*is_bool = 0;\n+\treturn -1;\n+}\n+\n+int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n+{\n+\tint v = git_config_maybe_bool(name, value);\n+\tif (0 <= v)\n+\t\treturn v;\n \treturn git_config_int(name, value);\n }\n \ndiff --git a/builtin-log.c b/builtin-log.c\nindex 3100dc0..23c00f0 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -253,6 +253,16 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"log.date\"))\n \t\treturn git_config_string(&default_date_mode, var, value);\n \tif (!strcmp(var, \"log.decorate\")) {\n+\t\tswitch (git_config_maybe_bool(var, value)) {\n+\t\tcase 0:\n+\t\t\tdecoration_style = 0;\n+\t\t\treturn 0;\n+\t\tcase 1:\n+\t\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\t\t\treturn 0;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n \t\tif (!strcmp(value, \"full\"))\n \t\t\tdecoration_style = DECORATE_FULL_REFS;\n \t\telse if (!strcmp(value, \"short\"))\n"},{"id":"134829","messageId":"alpine.LNX.2.00.1002171427080.3414@vqena.qenxr.bet.am","threadId":"22683","inReplyTo":"7vljespt2l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Steven Drake","fromEmail":"sdrake@xnet.co.nz","sentAt":"2010-02-17T02:04:21Z","receivedAt":"2010-02-17T02:04:21Z","isPatch":true,"sender":{"key":"sdrake@xnet.co.nz","avatar":null},"body":"On Tue, 16 Feb 2010, Junio C Hamano wrote:\n\n> This needs some test to make sure that it triggers when configuration is\n> set, it doesn't when configuration is not set [...]\n\nDone get wat you mean?\n\n> [...] and it doesn't for commands\n> in log family when it shouldn't (most notably, format-patch).\n\nGood point, and looking at the code \"log.decorate\" only has an affect after\ncmd_log_init() is called, which is call by cmd_whatchanged(), cmd_show(), \ncmd_log_reflog() and cmd_log() so only those command are affected\n(notably not format-patch).\n\nHowever if thats not disirable, we could always add\n'whatchanged.decorate', 'show.decorate' and reflog.decorate'. \n \n> > +log.decorate::\n> > +\tPrint out the ref names of any commits that are shown by the log\n> > +\tcommand. If 'short' is specified, the ref name prefixes 'refs/heads/',\n> > +\t'refs/tags/' and 'refs/remotes/' will not be printed. If 'full' is\n> > +\tspecified, the full ref name (including prefix) will be printed.\n> > +\tThis is the same as the log commands '--decorate' option.\n> \n> This should be the same as --decorate option, so it should be possible to\n> set it as a boolean true to mean \"short\", i.e.\n> \n> \t[log]\n>         \tdecorate\n> \t\tdecorate = true\n> \n> should be treated exactly the same way as\n> \n> \t[log]\n>         \tdecorate = short\n\nI thought about that but did not want start adding git_config_XXX()\nfunctions, but you want to add git_config_maybe_bool() then I would agree\nwith add your patch on top (and you should do so).\n\nWhile on the subject of git_config I think die_bad_config() should be an\nextern (i.e. decleared in cache.h and a static function) so that it could\nbe used in git_XXX_config functions for handling error.  Something like:\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex f096eea..a41a7bb 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -264,6 +264,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\t\tdecoration_style = DECORATE_FULL_REFS;\n \t\telse if (!strcmp(value, \"short\"))\n \t\t\tdecoration_style = DECORATE_SHORT_REFS;\n+\t\telse\n+\t\t\tdie_bad_config(var);\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"log.showroot\")) {\n\n-- \nSteven\nUNIX is basically a simple operating system,\nbut you have to be a genius to understand the simplicity  --- dmr\n"},{"id":"134832","messageId":"7v635wimac.fsf@alter.siamese.dyndns.org","threadId":"22683","inReplyTo":"alpine.LNX.2.00.1002171427080.3414@vqena.qenxr.bet.am","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-17T03:16:59Z","receivedAt":"2010-02-17T03:16:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Drake <sdrake@xnet.co.nz> writes:\n\n> Good point, and looking at the code \"log.decorate\" only has an affect after\n> cmd_log_init() is called, which is call by cmd_whatchanged(), cmd_show(), \n> cmd_log_reflog() and cmd_log() so only those command are affected\n> (notably not format-patch).\n\nI was not worried about what your change does.  I am worried about\nprotecting what the code after your change currently does from future\nchanges done by other people while you are not actively watching the\npatches in flight on this list.\n\n> While on the subject of git_config I think die_bad_config() should be an\n> extern (i.e. decleared in cache.h and a static function) so that it could\n> be used in git_XXX_config functions for handling error.  Something like:\n>\n> diff --git a/builtin-log.c b/builtin-log.c\n> index f096eea..a41a7bb 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -264,6 +264,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n>  \t\t\tdecoration_style = DECORATE_FULL_REFS;\n>  \t\telse if (!strcmp(value, \"short\"))\n>  \t\t\tdecoration_style = DECORATE_SHORT_REFS;\n> +\t\telse\n> +\t\t\tdie_bad_config(var);\n\nWe generally avoid doing this, as we may later want to add different\nvalues to \"log.decorate\", and keep the older git working as if nothing is\nspecified, rather than barfing, so that people can access the same\nrepository, perhaps over NFS, from different machines with varying vintage\nof git.\n"},{"id":"134842","messageId":"alpine.LNX.2.00.1002171950040.8560@vqena.qenxr.bet.am","threadId":"22683","inReplyTo":"7v635wimac.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Steven Drake","fromEmail":"sdrake@xnet.co.nz","sentAt":"2010-02-17T06:55:12Z","receivedAt":"2010-02-17T06:55:12Z","isPatch":true,"sender":{"key":"sdrake@xnet.co.nz","avatar":null},"body":"On Tue, 16 Feb 2010, Junio C Hamano wrote:\n\n> I was not worried about what your change does.  I am worried about\n> protecting what the code after your change currently does from future\n> changes done by other people while you are not actively watching the\n> patches in flight on this list.\nOk, I'll send a new patch that should be a lot better shorty.\n\nHave you commited the git_config_maybe_bool() code?\n\n> We generally avoid doing this, as we may later want to add different\n> values to \"log.decorate\", and keep the older git working as if nothing is\n> specified, rather than barfing, so that people can access the same\n> repository, perhaps over NFS, from different machines with varying vintage\n> of git.\nGood point.\n \nBy the way is it alright to send patches that use inbody-headers and/or \nscissors?\n-- \nSteven\n"},{"id":"134843","messageId":"36ca99e91002162342v2f151962p4d8f85f06c32205f@mail.gmail.com","threadId":"22683","inReplyTo":"7vljespt2l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-02-17T07:42:08Z","receivedAt":"2010-02-17T07:42:08Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Feb 17, 2010 at 02:08, Junio C Hamano <gitster@pobox.com> wrote:\n> diff --git a/config.c b/config.c\n> index 6963fbe..6642d30 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -322,9 +322,8 @@ unsigned long git_config_ulong(const char *name, const char *value)\n>        return ret;\n>  }\n>\n> -int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n> +int git_config_maybe_bool(const char *name, const char *value)\n>  {\n> -       *is_bool = 1;\n>        if (!value)\n>                return 1;\n>        if (!*value)\n> @@ -333,7 +332,14 @@ int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n>                return 1;\n>        if (!strcasecmp(value, \"false\") || !strcasecmp(value, \"no\") || !strcasecmp(value, \"off\"))\n>                return 0;\n> -       *is_bool = 0;\n> +       return -1;\n> +}\n> +\n> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n> +{\n> +       int v = git_config_maybe_bool(name, value);\n> +       if (0 <= v)\n> +               return v;\n>        return git_config_int(name, value);\n>  }\nWhat happened with the is_bool parameter?\n\nBert\n"},{"id":"134844","messageId":"7vk4uc70nh.fsf_-_@alter.siamese.dyndns.org","threadId":"22683","inReplyTo":"36ca99e91002162342v2f151962p4d8f85f06c32205f@mail.gmail.com","subject":"Re* [PATCH] Add `log.decorate' configuration variable.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-17T07:59:46Z","receivedAt":"2010-02-17T07:59:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n>> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n>> +{\n>> +       int v = git_config_maybe_bool(name, value);\n>> +       if (0 <= v)\n>> +               return v;\n>>        return git_config_int(name, value);\n>>  }\n> What happened with the is_bool parameter?\n\nGood eyes.  That was why it was \"Perhaps something like this\" without any\nserious commit message ;-).\n\nHow about this?\n\n-- >8 --\nSubject: git_config_maybe_bool()\n\nSome configuration variables can take boolean values in addition to\nenumeration specific to them.  Introduce git_config_maybe_bool() that\nreturns 0 or 1 if the given value is boolean, or -1 if not, so that\na parser for such a variable can check for boolean first and then\nparse other kinds of values as a fallback.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h  |    1 +\n config.c |   21 +++++++++++++++++----\n 2 files changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d478eff..dd3be0a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -924,6 +924,7 @@ extern int git_config_int(const char *, const char *);\n extern unsigned long git_config_ulong(const char *, const char *);\n extern int git_config_bool_or_int(const char *, const char *, int *);\n extern int git_config_bool(const char *, const char *);\n+extern int git_config_maybe_bool(const char *, const char *);\n extern int git_config_string(const char **, const char *, const char *);\n extern int git_config_pathname(const char **, const char *, const char *);\n extern int git_config_set(const char *, const char *);\ndiff --git a/config.c b/config.c\nindex 6963fbe..64e41be 100644\n--- a/config.c\n+++ b/config.c\n@@ -322,17 +322,30 @@ unsigned long git_config_ulong(const char *name, const char *value)\n \treturn ret;\n }\n \n-int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n+int git_config_maybe_bool(const char *name, const char *value)\n {\n-\t*is_bool = 1;\n \tif (!value)\n \t\treturn 1;\n \tif (!*value)\n \t\treturn 0;\n-\tif (!strcasecmp(value, \"true\") || !strcasecmp(value, \"yes\") || !strcasecmp(value, \"on\"))\n+\tif (!strcasecmp(value, \"true\")\n+\t    || !strcasecmp(value, \"yes\")\n+\t    || !strcasecmp(value, \"on\"))\n \t\treturn 1;\n-\tif (!strcasecmp(value, \"false\") || !strcasecmp(value, \"no\") || !strcasecmp(value, \"off\"))\n+\tif (!strcasecmp(value, \"false\")\n+\t    || !strcasecmp(value, \"no\")\n+\t    || !strcasecmp(value, \"off\"))\n \t\treturn 0;\n+\treturn -1;\n+}\n+\n+int git_config_bool_or_int(const char *name, const char *value, int *is_bool)\n+{\n+\tint v = git_config_maybe_bool(name, value);\n+\tif (0 <= v) {\n+\t\t*is_bool = 1;\n+\t\treturn v;\n+\t}\n \t*is_bool = 0;\n \treturn git_config_int(name, value);\n }\n"},{"id":"134848","messageId":"7v635w5ldv.fsf@alter.siamese.dyndns.org","threadId":"22683","inReplyTo":"alpine.LNX.2.00.1002171950040.8560@vqena.qenxr.bet.am","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-17T08:14:52Z","receivedAt":"2010-02-17T08:14:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Drake <sdrake@xnet.co.nz> writes:\n\n> Have you commited the git_config_maybe_bool() code?\n\nNot yet, but now you mention it, it probably is a good idea to make the\n\"maybe\" part a separate patch, independent from log.decorate.  It should\nbe useful elsewhere, I guess.\n\n> By the way is it alright to send patches that use inbody-headers and/or \n> scissors?\n\nIf used judiciously, i.e. when it makes it easier to follow the\ndiscussion.  It would sometimes make an important patch more likely to get\nburied in a deep thread, though.\n"},{"id":"134874","messageId":"20100217184142.GD2251@book.hvoigt.net","threadId":"22683","inReplyTo":"alpine.LNX.2.00.1002171239430.2477@vqena.qenxr.bet.am","subject":"Re: [PATCH] Add `log.decorate' configuration variable.","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-17T18:41:43Z","receivedAt":"2010-02-17T18:41:43Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Wed, Feb 17, 2010 at 12:39:52PM +1300, Steven Drake wrote:\n> This alows the 'git-log --decorate' to be enabled by default so that normal\n> log outout contains ant ref names of commits that are shown.\n\nI implemented the same option once but discarded the patch because of\nissues with gitk. If it is enabled you can not use gitk anymore thats\nwhy it was not useful to me because I switch between the two tools\nregularly.\n\nMaybe you have an idea how to avoid this?\n\ncheers Heiko\n"}]}