{"thread":{"id":"38595","subject":"[Q] should \"color.*.<slot> = normal\" emit nothing?","startedAt":"2015-02-18T21:03:06Z","lastAt":"2015-03-04T21:33:23Z","messageCount":16,"participants":["Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"256314","messageId":"xmqqzj8b0w6t.fsf@gitster.dls.corp.google.com","threadId":"38595","inReplyTo":null,"subject":"[Q] should \"color.*.<slot> = normal\" emit nothing?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T21:03:06Z","receivedAt":"2015-02-18T21:03:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you wanted to paint the HEAD decoration as the same color as the\nbody text (primarily because cyan is too faint on a black-on-white\nterminal to be readable) you would not want to say\n\n    [color \"decorate\"]\n        head = black\n\nbecause that you would not be able to reuse same configuration on\na white-on-black terminal.  I would naively expect\n\n    [color \"decorate\"]\n        head = normal\n\nto work, but it does not.  I notice that we have these definitions\nin color.h:\n\n    #define GIT_COLOR_NORMAL        \"\"\n    #define GIT_COLOR_RESET         \"\\033[m\"\n    #define GIT_COLOR_BOLD          \"\\033[1m\"\n    #define GIT_COLOR_RED           \"\\033[31m\"\n    #define GIT_COLOR_GREEN         \"\\033[32m\"\n    ...\n\nAs a workaround, I ended up doing this:\n\n    [color \"decorate\"]\n        head = reset\n\nwhich should work OK.  But I have a feeling that the definition of\nour \"normal\" may want to do the \"reset\", not \"no-op\" like we\ncurrently do.\n\nComments?\n"},{"id":"256316","messageId":"xmqqpp9628tl.fsf@gitster.dls.corp.google.com","threadId":"38595","inReplyTo":"xmqqzj8b0w6t.fsf@gitster.dls.corp.google.com","subject":"[PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T21:44:54Z","receivedAt":"2015-02-18T21:44:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you wanted to paint the HEAD and local branch name in the same\ncolor as the body text (perhaps because cyan and green are too faint\non a black-on-white terminal to be readable), you would not want to\nhave to say\n\n    [color \"decorate\"]\n        head = black\n        branch = black\n\nbecause that you would not be able to reuse same configuration on a\nwhite-on-black terminal.  You would naively expect\n\n    [color \"decorate\"]\n        head = normal\n\tbranch = normal\n\nto work, but unfortunately it does not.  It paints the string \"HEAD\"\nand the branch name in the same color as the opening parenthesis or\ncomma between the decoration elements when showing output like this:\n\n    $ git show -s --decorate\n    commit f3f407747c1cce420ae4b4857c4a6806efe38680 (HEAD, master)\n    ...\n\nThis is because the code forgets to reset the color after printing\nthe \"prefix\" in its own color.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Answering to myself ...\n\n log-tree.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 7f0890e..53bb526 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -195,6 +195,7 @@ void format_decorations_extended(struct strbuf *sb,\n \twhile (decoration) {\n \t\tstrbuf_addstr(sb, color_commit);\n \t\tstrbuf_addstr(sb, prefix);\n+\t\tstrbuf_addstr(sb, color_reset);\n \t\tstrbuf_addstr(sb, decorate_get_color(use_color, decoration->type));\n \t\tif (decoration->type == DECORATION_REF_TAG)\n \t\t\tstrbuf_addstr(sb, \"tag: \");\n"},{"id":"256321","messageId":"20150218230728.GA17302@peff.net","threadId":"38595","inReplyTo":"xmqqpp9628tl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-18T23:07:29Z","receivedAt":"2015-02-18T23:07:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 18, 2015 at 01:44:54PM -0800, Junio C Hamano wrote:\n\n> If you wanted to paint the HEAD and local branch name in the same\n> color as the body text (perhaps because cyan and green are too faint\n> on a black-on-white terminal to be readable), you would not want to\n> have to say\n> \n>     [color \"decorate\"]\n>         head = black\n>         branch = black\n> \n> because that you would not be able to reuse same configuration on a\n> white-on-black terminal.  You would naively expect\n> \n>     [color \"decorate\"]\n>         head = normal\n> \tbranch = normal\n> \n> to work, but unfortunately it does not.  It paints the string \"HEAD\"\n> and the branch name in the same color as the opening parenthesis or\n> comma between the decoration elements when showing output like this:\n> \n>     $ git show -s --decorate\n>     commit f3f407747c1cce420ae4b4857c4a6806efe38680 (HEAD, master)\n>     ...\n> \n> This is because the code forgets to reset the color after printing\n> the \"prefix\" in its own color.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n\nYeah, I think this is a good fix. I had a vague feeling that we may have\ndone this on purpose to let the decoration color \"inherit\" from the\nexisting colors for backwards compatibility, but I don't think that\ncould ever have worked (since color.decorate.* never defaulted to\n\"normal\"). And I couldn't find anything on the list. I think I am\nprobably thinking of color.diff.func, which faced a similar issue.\n\nAlso, for your amusement:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/191102/focus=191118\n\nBelieve it or not, this was actually an item on my todo list, which is\nperhaps a commentary on how sad and unrealistic my todo list is. But I'm\ncrossing it off anyway. Task accomplished!\n\n-Peff\n"},{"id":"256358","messageId":"xmqqvbixzsnv.fsf@gitster.dls.corp.google.com","threadId":"38595","inReplyTo":"20150218230728.GA17302@peff.net","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-19T18:02:12Z","receivedAt":"2015-02-19T18:02:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, I think this is a good fix. I had a vague feeling that we may have\n> done this on purpose to let the decoration color \"inherit\" from the\n> existing colors for backwards compatibility, but I don't think that\n> could ever have worked (since color.decorate.* never defaulted to\n> \"normal\").\n\nHmph, but that $gmane/191118 talks about giving bold to commit-color\nand then expecting for decors to inherit the boldness, a wish I can\nunderstand.  But I do not necessarily agree with it---it relies on\nthat after \"<commit-color>(\" and \"<commit-color>, \" there is no reset,\nwhich is not how everything else works.\n\nSo this change at least needs to come with an explanation to people\nwho are used to and took advantage of this color attribute leakage,\ndefinitely in the log message and preferrably to the documentation\nthat covers all the color.*.<slot> settings, I think.\n\nThanks.\n"},{"id":"256382","messageId":"20150220014230.GA16124@peff.net","threadId":"38595","inReplyTo":"xmqqvbixzsnv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-20T01:42:31Z","receivedAt":"2015-02-20T01:42:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 19, 2015 at 10:02:12AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Yeah, I think this is a good fix. I had a vague feeling that we may have\n> > done this on purpose to let the decoration color \"inherit\" from the\n> > existing colors for backwards compatibility, but I don't think that\n> > could ever have worked (since color.decorate.* never defaulted to\n> > \"normal\").\n> \n> Hmph, but that $gmane/191118 talks about giving bold to commit-color\n> and then expecting for decors to inherit the boldness, a wish I can\n> understand.  But I do not necessarily agree with it---it relies on\n> that after \"<commit-color>(\" and \"<commit-color>, \" there is no reset,\n> which is not how everything else works.\n\nI don't see anybody actually _wanting_ the inheritance. It is mentioned\nmerely as an observation. So yeah, we would break anybody who does:\n\n  [color \"diff\"]\n  commit = blue\n\n  [color \"decorate\"]\n  branch = normal\n  remoteBranch = normal\n  tag = normal\n  stash = normal\n  HEAD = normal\n\nand expects the \"blue\" to persist automatically.\n\nBut given that this behaves in the opposite way of every other part of\ngit's color handling, I think we can call it a bug, and people doing\nthat are crazy (they should s/normal/blue/ in the latter config).\n\n> So this change at least needs to come with an explanation to people\n> who are used to and took advantage of this color attribute leakage,\n> definitely in the log message and preferrably to the documentation\n> that covers all the color.*.<slot> settings, I think.\n\nI'd agree it is worth a mention in the log (and possibly release notes),\nbut I don't think it is worth polluting the documentation forever\n(though explaining that we never inherit might be worth doing, and that\nis perhaps what you meant).\n\n-Peff\n"},{"id":"256446","messageId":"xmqqwq3cxjwg.fsf@gitster.dls.corp.google.com","threadId":"38595","inReplyTo":"20150220014230.GA16124@peff.net","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-20T23:06:39Z","receivedAt":"2015-02-20T23:06:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'd agree it is worth a mention in the log (and possibly release notes),\n> but I don't think it is worth polluting the documentation forever\n> (though explaining that we never inherit might be worth doing, and that\n> is perhaps what you meant).\n\nYes, I do not know how well the attached will render, but something\nalong the lines of this patch is what I had in mind.\n\n-- >8 --\nSubject: config.txt: spell out how certain typed values are written\n\nMany variables have values that are not arbitrary strings and there\nare ways to spell these values of certain types.  The way to spell\ncolors was described in a section for color.branch.<slot> and other\nvariables refered to that section, which was technically wrong, but\nwas a bit awkward.\n\nI didn't attempt to de-dup descriptions for boolean and integer\nvalued variables in this change, but somebody may want to read\nthe existing descriptions of these variables over and drop them\nas necessary.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 58 +++++++++++++++++++++++++++++++++++-------------\n 1 file changed, 42 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 097fdd4..171287e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -126,6 +126,45 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your $HOME directory\n \n+Values\n+~~~~~~\n+\n+Values of many variables are treated as a simple string, but there\n+are variables that take values of specific types and there are rules\n+as to how to spell them.\n+\n+boolean::\n+\tWhen a variable is said to take a boolean value, many\n+\tsynonyms are accepted for 'true' and 'false'.\n+\ttrue;; Boolean true can be spelled as `yes`, `on`, `true`,\n+\t    or `1`.  Also, a variable defined without `= <value>`\n+\t    is taken as true.\n+\tfalse;; Boolean false can be spelled as `no`, `off`,\n+\t    `false`, or `0`.\n+\n+integer::\n+\tThe value for many variables that specify various sizes can\n+\tbe suffixed with `k`, `M`,... to mean \"scale the number by\n+\t1024\", \"by 1024x1024\", etc.\n+\n+color::\n+\tThe value for a variables that takes a color is a list of\n+\tcolors (at most two) and attributes (at most one), separated\n+\tby spaces.  The colors accepted are `normal`, `black`,\n+\t`red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and\n+\t`white`; the attributes are `bold`, `dim`, `ul`, `blink` and\n+\t`reverse`.  The first color given is the foreground; the\n+\tsecond is the background.  The position of the attribute, if\n+\tany, doesn't matter.\n++\n+The attributes are meant to be reset at the beginning of each item\n+in the colored output, so setting color.decorate.branch to `black`\n+will paint that branch name in a plain `black`, even if the previous\n+thing on the same output line (e.g. opening parenthesis before the\n+list of branch names in `log --decorate` output) is set to be painted\n+with `bold` or some other attribute.\n+\n+\n Variables\n ~~~~~~~~~\n \n@@ -817,14 +856,6 @@ color.branch.<slot>::\n \t`remote` (a remote-tracking branch in refs/remotes/),\n \t`upstream` (upstream tracking branch), `plain` (other\n \trefs).\n-+\n-The value for these configuration variables is a list of colors (at most\n-two) and attributes (at most one), separated by spaces.  The colors\n-accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`,\n-`magenta`, `cyan` and `white`; the attributes are `bold`, `dim`, `ul`,\n-`blink` and `reverse`.  The first color given is the foreground; the\n-second is the background.  The position of the attribute, if any,\n-doesn't matter.\n \n color.diff::\n \tWhether to use ANSI escape sequences to add color to patches.\n@@ -844,8 +875,7 @@ color.diff.<slot>::\n \tof `plain` (context text), `meta` (metainformation), `frag`\n \t(hunk header), 'func' (function in hunk header), `old` (removed lines),\n \t`new` (added lines), `commit` (commit headers), or `whitespace`\n-\t(highlighting whitespace errors). The values of these variables may be\n-\tspecified as in color.branch.<slot>.\n+\t(highlighting whitespace errors).\n \n color.decorate.<slot>::\n \tUse customized color for 'git log --decorate' output.  `<slot>` is one\n@@ -878,8 +908,6 @@ color.grep.<slot>::\n \tseparators between fields on a line (`:`, `-`, and `=`)\n \tand between hunks (`--`)\n --\n-+\n-The values of these variables may be specified as in color.branch.<slot>.\n \n color.interactive::\n \tWhen set to `always`, always use colors for interactive prompts\n@@ -892,8 +920,7 @@ color.interactive.<slot>::\n \tUse customized color for 'git add --interactive' and 'git clean\n \t--interactive' output. `<slot>` may be `prompt`, `header`, `help`\n \tor `error`, for four distinct types of normal output from\n-\tinteractive commands.  The values of these variables may be\n-\tspecified as in color.branch.<slot>.\n+\tinteractive commands.\n \n color.pager::\n \tA boolean to enable/disable colored output when the pager is in\n@@ -919,8 +946,7 @@ color.status.<slot>::\n \t`untracked` (files which are not tracked by Git),\n \t`branch` (the current branch), or\n \t`nobranch` (the color the 'no branch' warning is shown in, defaulting\n-\tto red). The values of these variables may be specified as in\n-\tcolor.branch.<slot>.\n+\tto red).\n \n color.ui::\n \tThis variable determines the default value for variables such\n"},{"id":"256461","messageId":"20150221062310.GB29665@peff.net","threadId":"38595","inReplyTo":"xmqqwq3cxjwg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-21T06:23:10Z","receivedAt":"2015-02-21T06:23:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 20, 2015 at 03:06:39PM -0800, Junio C Hamano wrote:\n\n> -- >8 --\n> Subject: config.txt: spell out how certain typed values are written\n> \n> Many variables have values that are not arbitrary strings and there\n> are ways to spell these values of certain types.  The way to spell\n> colors was described in a section for color.branch.<slot> and other\n> variables refered to that section, which was technically wrong, but\n> was a bit awkward.\n\nDid you mean \"not technically\" here?\n\nI think this patch is certainly an improvement with respect to the\ncolors. And I like that it organizes the types into one place using a\nlist, which is easier to scan when you are looking at a manpage. But...\n\n> +Values\n> +~~~~~~\n> +\n> +Values of many variables are treated as a simple string, but there\n> +are variables that take values of specific types and there are rules\n> +as to how to spell them.\n> +\n> +boolean::\n> +\tWhen a variable is said to take a boolean value, many\n> +\tsynonyms are accepted for 'true' and 'false'.\n> +\ttrue;; Boolean true can be spelled as `yes`, `on`, `true`,\n> +\t    or `1`.  Also, a variable defined without `= <value>`\n> +\t    is taken as true.\n> +\tfalse;; Boolean false can be spelled as `no`, `off`,\n> +\t    `false`, or `0`.\n\nThis information is redundant with what is in the `Syntax` section:\n\n  The values following the equals sign in variable assign are all either\n  a string, an integer, or a boolean.  Boolean values may be given as\n  yes/no, 1/0, true/false or on/off.  Case is not significant in boolean\n  values, when converting value to the canonical form using '--bool'\n  type specifier; 'git config' will ensure that the output is \"true\" or\n  \"false\".\n\nI think that paragraph can go away in favor of what you've written. We\nshould mention case-insensitivity there. And the final sentence about\n`git-config` should go in the section `--bool` description of\n`git-config`.\n\nImmediately after that paragraph we discuss string values and quoting.\nTechnically those quoting rules apply to all values (e.g., colors, which\nare just strings with special meaning), but I it may make sense to put\nthem in a \"strong::\" bullet here.\n\n> +integer::\n> +\tThe value for many variables that specify various sizes can\n> +\tbe suffixed with `k`, `M`,... to mean \"scale the number by\n> +\t1024\", \"by 1024x1024\", etc.\n\nI had a feeling we only did this for ulong's, but I checked the code and\nwe do indeed handle handle unit sizes everywhere. Is it worth mentioning\nrange limits here? I think since c7c377d83f4 it is probably not a big\ndeal. \"git config --int\" uses 64-bit integers everywhere. Internally we\nuse \"unsigned long\" for big things. There are still some uses of \"int\"\ninternally, but only for things that should obviously be small. And in\nany case we complain if there is overflow. So probably it is not\nsomething users need to think about.\n\n-Peff\n"},{"id":"256463","messageId":"xmqq4mqfyc3e.fsf@gitster.dls.corp.google.com","threadId":"38595","inReplyTo":"20150221062310.GB29665@peff.net","subject":"Re: [PATCH] log --decorate: do not leak \"commit\" color into the next item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-21T07:09:57Z","receivedAt":"2015-02-21T07:09:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Feb 20, 2015 at 03:06:39PM -0800, Junio C Hamano wrote:\n>\n>> -- >8 --\n>> Subject: config.txt: spell out how certain typed values are written\n>> \n>> Many variables have values that are not arbitrary strings and there\n>> are ways to spell these values of certain types.  The way to spell\n>> colors was described in a section for color.branch.<slot> and other\n>> variables refered to that section, which was technically wrong, but\n>> was a bit awkward.\n>\n> Did you mean \"not technically\" here?\n\nYes.\n\n> I think this patch is certainly an improvement with respect to the\n> colors. And I like that it organizes the types into one place using a\n> list, which is easier to scan when you are looking at a manpage. But...\n>\n>> +Values\n>> +~~~~~~\n>> +\n>> +Values of many variables are treated as a simple string, but there\n>> +are variables that take values of specific types and there are rules\n>> +as to how to spell them.\n>> +\n>> +boolean::\n>> +\tWhen a variable is said to take a boolean value, many\n>> +\tsynonyms are accepted for 'true' and 'false'.\n>> +\ttrue;; Boolean true can be spelled as `yes`, `on`, `true`,\n>> +\t    or `1`.  Also, a variable defined without `= <value>`\n>> +\t    is taken as true.\n>> +\tfalse;; Boolean false can be spelled as `no`, `off`,\n>> +\t    `false`, or `0`.\n>\n> This information is redundant with what is in the `Syntax` section:\n> ...\n\nHeh, thanks.  That is what I wanted to say when I said \"other types\nmay need de-duping\" ;-).\n"},{"id":"257022","messageId":"1425504803-16039-1-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"xmqqpp9628tl.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 0/7] Fix leak of color/attributes in \"git log --decorate\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:16Z","receivedAt":"2015-03-04T21:33:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In \"git log --decorate\", you would see the commit header like this:\n\n    commit ... (HEAD, jc/decorate-leaky-separator-color)\n\nwhere \"commit ... (\" is painted in color.diff.commit, \"HEAD\" in\ncolor.decorate.head, \", \" in color.diff.commit, the branch name in\ncolor.decorate.branch and then closing \")\" in color.diff.commit.\n\nHowever, setting color.decorate.head to normal does not paint \"HEAD\"\nin the \"normal\" color you have for your terminal.  It just uses the\nsame color it used to paint the \"(\", i.e. color.diff.commit.\n\nFixing this was a simple one-liner; the code forgot to reset the\nterminal attributes before moving on to the next item.\n\nIt however turns out that the existing documentation was rather\nmessy and I ended up doing some preparatory clean-up on the\ndocumentation around how configuration variables are explained\nbefore updating the documentation to clarify that 'normal' ought to\nwork (in other words, the colors and attributes are not cumulative).\n\nI am reasonably happy with the result, and I can go with or without\n[6/7].\n\nThe previous round starts at $gmane/264065 [*1*]\n\nJunio C Hamano (7):\n  Documentation/config.txt: avoid unnecessary negation\n  Documentation/config.txt: explain multi-valued variables once\n  Documentation/config.txt: describe the structure first and then meaning\n  Documentation/config.txt: have a separate \"Values\" section\n  Documentation/config.txt: describe 'color' value type in the \"Values\" section\n  Documentation/config.txt: simplify boolean description in the syntax section\n  log --decorate: do not leak \"commit\" color into the next item\n\n Documentation/config.txt         | 111 ++++++++++++++++++++++++---------------\n log-tree.c                       |   1 +\n t/t4207-log-decoration-colors.sh |  16 +++---\n 3 files changed, 77 insertions(+), 51 deletions(-)\n\n[Footnote]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/264063/focus=264065\n    http://mid.gmane.org/xmqqpp9628tl.fsf@gitster.dls.corp.google.com\n\n-- \n2.3.1-316-g7c93423\n"},{"id":"257021","messageId":"1425504803-16039-2-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/7] Documentation/config.txt: avoid unnecessary negation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:17Z","receivedAt":"2015-03-04T21:33:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Section names and variable names are both case-insensitive, but one\nis described as \"not case sensitive\".  Use \"case-insensitive\" for\nboth.\n\nInstead of saying \"... have to be escaped\" without telling what that\nescaping achieves, state it in a more positive way, i.e. \"... can be\nincluded by escaping\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 097fdd4..dbe7035 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -25,7 +25,7 @@ blank lines are ignored.\n \n The file consists of sections and variables.  A section begins with\n the name of the section in square brackets and continues until the next\n-section begins.  Section names are not case sensitive.  Only alphanumeric\n+section begins.  Section names are case-insensitive.  Only alphanumeric\n characters, `-` and `.` are allowed in section names.  Each variable\n must belong to some section, which means that there must be a section\n header before the first setting of a variable.\n@@ -40,8 +40,8 @@ in the section header, like in the example below:\n --------\n \n Subsection names are case sensitive and can contain any characters except\n-newline (doublequote `\"` and backslash have to be escaped as `\\\"` and `\\\\`,\n-respectively).  Section headers cannot span multiple\n+newline (doublequote `\"` and backslash can be included by escaping them\n+as `\\\"` and `\\\\`, respectively).  Section headers cannot span multiple\n lines.  Variables may belong directly to a section or to a given subsection.\n You can have `[section]` if you have `[section \"subsection\"]`, but you\n don't need to.\n-- \n2.3.1-316-g7c93423\n"},{"id":"257025","messageId":"1425504803-16039-3-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/7] Documentation/config.txt: explain multi-valued variables once","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:18Z","receivedAt":"2015-03-04T21:33:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The syntax section repeats what the preamble explained already.\nThat a variable can have multiple values is more about what a\nvariable is than the syntax of the file.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex dbe7035..405bf25 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -14,7 +14,8 @@ the fully qualified variable name of the variable itself is the last\n dot-separated segment and the section name is everything before the last\n dot. The variable names are case-insensitive, allow only alphanumeric\n characters and `-`, and must start with an alphabetic character.  Some\n-variables may appear multiple times.\n+variables may appear multiple times; we say then that the variable is\n+multivalued.\n \n Syntax\n ~~~~~~\n@@ -56,9 +57,7 @@ header) are recognized as setting variables, in the form\n 'name = value'.  If there is no equal sign on the line, the entire line\n is taken as 'name' and the variable is recognized as boolean \"true\".\n The variable names are case-insensitive, allow only alphanumeric characters\n-and `-`, and must start with an alphabetic character.  There can be more\n-than one value for a given variable; we say then that the variable is\n-multivalued.\n+and `-`, and must start with an alphabetic character.\n \n Leading and trailing whitespace in a variable value is discarded.\n Internal whitespace within a variable value is retained verbatim.\n-- \n2.3.1-316-g7c93423\n"},{"id":"257023","messageId":"1425504803-16039-4-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/7] Documentation/config.txt: describe the structure first and then meaning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:19Z","receivedAt":"2015-03-04T21:33:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A line can be continued via a backquote-LF and can be chomped at a\ncomment character.  But that is not specific to string-typed values.\nIt is common to all, just like unquoted leading and trailing\nwhitespaces are stripped and inter-word spacing are retained.\n\nMove the description around and desribe these structural rules\nfirst, then introduce the double-quote facility as a way to override\nthem, and finally mention various types of values.\n\nNote that these structural rules only apply to the value part of the\nconfiguration file.  E.g.\n\n    [aSection] \\\n        name \\\n\t= value\n\ndoes not work, because the rules kick in only after seeing \"name =\".\nBoth the original and the updated text are phrased in an awkward way\nby singling out the \"value\" part of the line because of this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 31 +++++++++++++++----------------\n 1 file changed, 15 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 405bf25..1444614 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -59,32 +59,31 @@ is taken as 'name' and the variable is recognized as boolean \"true\".\n The variable names are case-insensitive, allow only alphanumeric characters\n and `-`, and must start with an alphabetic character.\n \n-Leading and trailing whitespace in a variable value is discarded.\n-Internal whitespace within a variable value is retained verbatim.\n+A line that defines a value can be continued to the next line by\n+ending it with a `\\`; the backquote and the end-of-line are\n+stripped.  Leading whitespaces after 'name =', the remainder of the\n+line after the first comment character '#' or ';', and trailing\n+whitespaces of the line are discarded unless they are enclosed in\n+double quotes.  Internal whitespaces within the value are retained\n+verbatim.\n \n-The values following the equals sign in variable assign are all either\n-a string, an integer, or a boolean.  Boolean values may be given as yes/no,\n-1/0, true/false or on/off.  Case is not significant in boolean values, when\n-converting value to the canonical form using '--bool' type specifier;\n-'git config' will ensure that the output is \"true\" or \"false\".\n-\n-String values may be entirely or partially enclosed in double quotes.\n-You need to enclose variable values in double quotes if you want to\n-preserve leading or trailing whitespace, or if the variable value contains\n-comment characters (i.e. it contains '#' or ';').\n-Double quote `\"` and backslash `\\` characters in variable values must\n-be escaped: use `\\\"` for `\"` and `\\\\` for `\\`.\n+Inside double quotes, double quote `\"` and backslash `\\` characters\n+must be escaped: use `\\\"` for `\"` and `\\\\` for `\\`.\n \n The following escape sequences (beside `\\\"` and `\\\\`) are recognized:\n `\\n` for newline character (NL), `\\t` for horizontal tabulation (HT, TAB)\n and `\\b` for backspace (BS).  No other char escape sequence, nor octal\n char sequences are valid.\n \n-Variable values ending in a `\\` are continued on the next line in the\n-customary UNIX fashion.\n+The values following the equals sign in variable assign are all either\n+a string, an integer, or a boolean.  Boolean values may be given as yes/no,\n+1/0, true/false or on/off.  Case is not significant in boolean values, when\n+converting value to the canonical form using '--bool' type specifier;\n+'git config' will ensure that the output is \"true\" or \"false\".\n \n Some variables may require a special value format.\n \n+\n Includes\n ~~~~~~~~\n \n-- \n2.3.1-316-g7c93423\n"},{"id":"257026","messageId":"1425504803-16039-5-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 4/7] Documentation/config.txt: have a separate \"Values\" section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:20Z","receivedAt":"2015-03-04T21:33:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The various types of values set to the configuration variables\ndeserve more than a brief footnote mention in the syntax section,\nand it will be more so after the later steps of this clean up\neffort.\n\nMove the mention of booleans from the syntax section to this new\nsection, and describe how human-readble integers can be spelled with\nscaling there.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 39 +++++++++++++++++++++++++++++++--------\n 1 file changed, 31 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1444614..7be608b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -75,14 +75,6 @@ The following escape sequences (beside `\\\"` and `\\\\`) are recognized:\n and `\\b` for backspace (BS).  No other char escape sequence, nor octal\n char sequences are valid.\n \n-The values following the equals sign in variable assign are all either\n-a string, an integer, or a boolean.  Boolean values may be given as yes/no,\n-1/0, true/false or on/off.  Case is not significant in boolean values, when\n-converting value to the canonical form using '--bool' type specifier;\n-'git config' will ensure that the output is \"true\" or \"false\".\n-\n-Some variables may require a special value format.\n-\n \n Includes\n ~~~~~~~~\n@@ -124,6 +116,37 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your $HOME directory\n \n+\n+Values\n+~~~~~~\n+\n+Values of many variables are treated as a simple string, but there\n+are variables that take values of specific types and there are rules\n+as to how to spell them.\n+\n+boolean::\n+\n+       When a variable is said to take a boolean value, many\n+       synonyms are accepted for 'true' and 'false'; these are all\n+       case-insensitive.\n+\n+       true;; Boolean true can be spelled as `yes`, `on`, `true`,\n+\t\tor `1`.  Also, a variable defined without `= <value>`\n+\t\tis taken as true.\n+\n+       false;; Boolean false can be spelled as `no`, `off`,\n+\t\t`false`, or `0`.\n++\n+When converting value to the canonical form using '--bool' type\n+specifier; 'git config' will ensure that the output is \"true\" or\n+\"false\" (spelled in lowercase).\n+\n+integer::\n+       The value for many variables that specify various sizes can\n+       be suffixed with `k`, `M`,... to mean \"scale the number by\n+       1024\", \"by 1024x1024\", etc.\n+\n+\n Variables\n ~~~~~~~~~\n \n-- \n2.3.1-316-g7c93423\n"},{"id":"257024","messageId":"1425504803-16039-6-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 5/7] Documentation/config.txt: describe 'color' value type in the \"Values\" section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:21Z","receivedAt":"2015-03-04T21:33:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of describing it for color.branch.<slot> and have everybody\nelse refer to it, explain how colors are spelled in \"Values\" section\nupfront.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 29 +++++++++++++----------------\n 1 file changed, 13 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7be608b..c40bf4a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -146,6 +146,16 @@ integer::\n        be suffixed with `k`, `M`,... to mean \"scale the number by\n        1024\", \"by 1024x1024\", etc.\n \n+color::\n+       The value for a variables that takes a color is a list of\n+       colors (at most two) and attributes (at most one), separated\n+       by spaces.  The colors accepted are `normal`, `black`,\n+       `red`, `green`, `yellow`, `blue`, `magenta`, `cyan` and\n+       `white`; the attributes are `bold`, `dim`, `ul`, `blink` and\n+       `reverse`.  The first color given is the foreground; the\n+       second is the background.  The position of the attribute, if\n+       any, doesn't matter.\n+\n \n Variables\n ~~~~~~~~~\n@@ -838,14 +848,6 @@ color.branch.<slot>::\n \t`remote` (a remote-tracking branch in refs/remotes/),\n \t`upstream` (upstream tracking branch), `plain` (other\n \trefs).\n-+\n-The value for these configuration variables is a list of colors (at most\n-two) and attributes (at most one), separated by spaces.  The colors\n-accepted are `normal`, `black`, `red`, `green`, `yellow`, `blue`,\n-`magenta`, `cyan` and `white`; the attributes are `bold`, `dim`, `ul`,\n-`blink` and `reverse`.  The first color given is the foreground; the\n-second is the background.  The position of the attribute, if any,\n-doesn't matter.\n \n color.diff::\n \tWhether to use ANSI escape sequences to add color to patches.\n@@ -865,8 +867,7 @@ color.diff.<slot>::\n \tof `plain` (context text), `meta` (metainformation), `frag`\n \t(hunk header), 'func' (function in hunk header), `old` (removed lines),\n \t`new` (added lines), `commit` (commit headers), or `whitespace`\n-\t(highlighting whitespace errors). The values of these variables may be\n-\tspecified as in color.branch.<slot>.\n+\t(highlighting whitespace errors).\n \n color.decorate.<slot>::\n \tUse customized color for 'git log --decorate' output.  `<slot>` is one\n@@ -899,8 +900,6 @@ color.grep.<slot>::\n \tseparators between fields on a line (`:`, `-`, and `=`)\n \tand between hunks (`--`)\n --\n-+\n-The values of these variables may be specified as in color.branch.<slot>.\n \n color.interactive::\n \tWhen set to `always`, always use colors for interactive prompts\n@@ -913,8 +912,7 @@ color.interactive.<slot>::\n \tUse customized color for 'git add --interactive' and 'git clean\n \t--interactive' output. `<slot>` may be `prompt`, `header`, `help`\n \tor `error`, for four distinct types of normal output from\n-\tinteractive commands.  The values of these variables may be\n-\tspecified as in color.branch.<slot>.\n+\tinteractive commands.\n \n color.pager::\n \tA boolean to enable/disable colored output when the pager is in\n@@ -940,8 +938,7 @@ color.status.<slot>::\n \t`untracked` (files which are not tracked by Git),\n \t`branch` (the current branch), or\n \t`nobranch` (the color the 'no branch' warning is shown in, defaulting\n-\tto red). The values of these variables may be specified as in\n-\tcolor.branch.<slot>.\n+\tto red).\n \n color.ui::\n \tThis variable determines the default value for variables such\n-- \n2.3.1-316-g7c93423\n"},{"id":"257027","messageId":"1425504803-16039-7-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 6/7] Documentation/config.txt: simplify boolean description in the syntax section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:22Z","receivedAt":"2015-03-04T21:33:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The 'true' short-hand doesn't deserve a separate sentence; even our own\n\n    git config --bool foo.bar yes\n\nwould not produce it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c40bf4a..f1cf571 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -54,8 +54,8 @@ restrictions as section names.\n \n All the other lines (and the remainder of the line after the section\n header) are recognized as setting variables, in the form\n-'name = value'.  If there is no equal sign on the line, the entire line\n-is taken as 'name' and the variable is recognized as boolean \"true\".\n+'name = value' (or just 'name', which is a short-hand to say that\n+the variable is the boolean \"true\").\n The variable names are case-insensitive, allow only alphanumeric characters\n and `-`, and must start with an alphabetic character.\n \n-- \n2.3.1-316-g7c93423\n"},{"id":"257029","messageId":"1425504803-16039-8-git-send-email-gitster@pobox.com","threadId":"38595","inReplyTo":"1425504803-16039-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 7/7] log --decorate: do not leak \"commit\" color into the next item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-04T21:33:23Z","receivedAt":"2015-03-04T21:33:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In \"git log --decorate\", you would see the commit header like this:\n\n    commit ... (HEAD, jc/decorate-leaky-separator-color)\n\nwhere \"commit ... (\" is painted in color.diff.commit, \"HEAD\" in\ncolor.decorate.head, \", \" in color.diff.commit, the branch name in\ncolor.decorate.branch and then closing \")\" in color.diff.commit.\n\nIf you wanted to paint the HEAD and local branch name in the same\ncolor as the body text (perhaps because cyan and green are too faint\non a black-on-white terminal to be readable), you would not want to\nhave to say\n\n    [color \"decorate\"]\n        head = black\n        branch = black\n\nbecause that you would not be able to reuse same configuration on a\nwhite-on-black terminal.  You would naively expect\n\n    [color \"decorate\"]\n        head = normal\n\tbranch = normal\n\nto work, but unfortunately it does not.  It paints the string \"HEAD\"\nand the branch name in the same color as the opening parenthesis or\ncomma between the decoration elements.  This is because the code\nforgets to reset the color after printing the \"prefix\" in its own\ncolor.\n\nIt theoretically is possible that some people were expecting and\nrelying on that the attribute set as the \"diff.commit\" color, which\nis used to draw these opening parenthesis and inter-item comma, is\ninherited by the drawing of branch names, but it is not how the\ncoloring works everywhere else.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt         |  7 +++++++\n log-tree.c                       |  1 +\n t/t4207-log-decoration-colors.sh | 16 ++++++++--------\n 3 files changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f1cf571..6d65033 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -155,6 +155,13 @@ color::\n        `reverse`.  The first color given is the foreground; the\n        second is the background.  The position of the attribute, if\n        any, doesn't matter.\n++\n+The attributes are meant to be reset at the beginning of each item\n+in the colored output, so setting color.decorate.branch to `black`\n+will paint that branch name in a plain `black`, even if the previous\n+thing on the same output line (e.g. opening parenthesis before the\n+list of branch names in `log --decorate` output) is set to be\n+painted with `bold` or some other attribute.\n \n \n Variables\ndiff --git a/log-tree.c b/log-tree.c\nindex 1982631..11676d5 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -200,6 +200,7 @@ void format_decorations(struct strbuf *sb,\n \twhile (decoration) {\n \t\tstrbuf_addstr(sb, color_commit);\n \t\tstrbuf_addstr(sb, prefix);\n+\t\tstrbuf_addstr(sb, color_reset);\n \t\tstrbuf_addstr(sb, decorate_get_color(use_color, decoration->type));\n \t\tif (decoration->type == DECORATION_REF_TAG)\n \t\t\tstrbuf_addstr(sb, \"tag: \");\ndiff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh\nindex 925f577..6b8ad4f 100755\n--- a/t/t4207-log-decoration-colors.sh\n+++ b/t/t4207-log-decoration-colors.sh\n@@ -44,15 +44,15 @@ test_expect_success setup '\n '\n \n cat >expected <<EOF\n-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_HEAD}HEAD${c_reset}${c_commit},\\\n- ${c_tag}tag: v1.0${c_reset}${c_commit},\\\n- ${c_tag}tag: B${c_reset}${c_commit},\\\n- ${c_branch}master${c_reset}${c_commit})${c_reset} B\n-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A1${c_reset}${c_commit},\\\n- ${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1\n-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\\\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}${c_commit},\\\n+ ${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\\\n+ ${c_reset}${c_tag}tag: B${c_reset}${c_commit},\\\n+ ${c_reset}${c_branch}master${c_reset}${c_commit})${c_reset} B\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\\\n+ ${c_reset}${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\\\n  On master: Changes to A.t\n-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A${c_reset}${c_commit})${c_reset} A\n+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A\n EOF\n \n # We want log to show all, but the second parent to refs/stash is irrelevant\n-- \n2.3.1-316-g7c93423\n"}]}