Skip to content
  • Jeff King's avatar
    color_parse_mem: allow empty color spec · 55cccf4b
    Jeff King authored and Junio C Hamano's avatar Junio C Hamano committed
    Prior to c2f41bf5 (color.c: fix color_parse_mem() with
    value_len == 0, 2017-01-19), the empty string was
    interpreted as a color "reset". This was an accidental
    outcome, and that commit turned it into an error.
    
    However, scripts may pass the empty string as a default
    value to "git config --get-color" to disable color when the
    value is not defined. The git-add--interactive script does
    this. As a result, the script is unusable since c2f41bf5
    unless you have color.diff.plain defined (if it is defined,
    then we don't parse the empty default at all).
    
    Our test scripts didn't notice the recent breakage because
    they run without a terminal, and thus without color. They
    never hit this code path at all. And nobody noticed the
    original buggy "reset" behavior, because it was effectively
    a noop.
    
    Let's fix the code to have an empty color name produce an
    empty sequence of color codes. The tests need a few fixups:
    
      - we'll add a new test in t4026 to cover this case. But
        note that we need to tweak the color() helper. While
        we're there, let's factor out the literal ANSI ESC
        character. Otherwise it makes the diff quite hard to
        read.
    
      - we'll add a basic sanity-check in t4026 that "git add
        -p" works at all when color is enabled. That would have
        caught this bug, as well as any others that are specific
        to the color code paths.
    
      - 73c727d6
    
     (log --graph: customize the graph lines with
        config log.graphColors, 2017-01-19) added a test to
        t4202 that checks some "invalid" graph color config.
        Since ",, blue" before yielded only "blue" as valid, and
        now yields "empty, empty, blue", we don't match the
        expected output.
    
        One way to fix this would be to change the expectation
        to the empty color strings. But that makes the test much
        less interesting, since we show only two graph lines,
        both of which would be colorless.
    
        Since the empty-string case is now covered by t4026,
        let's remove them entirely here. They're just in the way
        of the primary thing the test is supposed to be
        checking.
    
    Signed-off-by: default avatarJeff King <peff@peff.net>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    55cccf4b