Re: [PATCH 17/20] abbrev: unify the handling of empty values

From: Martin Ågren <martin.agren@gmail.com>
Date: Jun 9, 2018, 14:24 UTC
Message-ID: <CAN0heSqrgUw1u8Ykk-9eGosWwFS0NFM7sLBe5Loo4cN=tkrXaA@mail.gmail.com>
In-Reply-To: <20180608224136.20220-18-avarab@gmail.com>

On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

> For no good reason the --abbrev= command-line option was less strict
> than the core.abbrev config option, which came down to the latter
> using git_config_int() which rejects an empty string, but the rest of
> the parsing using strtoul() which will convert it to 0.

It will still be less strict in that it accepts trailing garbage, e.g., `--abbrev=7a`. Probably ok to leave it at that in this series, but possibly useful to mention here that this only makes these options "less differently strict".

I also notice that the documentation of `--abbrev` starts with "Instead of showing the full 40-byte hexadecimal object name" which doesn't seem right. I get much shorter IDs and I can't see that I'd have any configuration causing this. Anyway, that might not be anything this series needs to do anything about.

> +               if (!strcmp(arg, ""))
> +                       die("--abbrev expects a value, got '%s'", arg);

> +               if (!strcmp(arg, ""))
> +                       return opterror(opt, "expects a value", 0);

> +               if (!strcmp(optarg, ""))
> +                       die("--abbrev expects a value, got '%s'", optarg);

> +       test_must_fail git branch -v --abbrev= 2>stderr &&
> +       test_i18ngrep "expects a value" stderr &&

> +       test_must_fail git log --abbrev= -1 --pretty=format:%h 2>stderr &&
> +       test_i18ngrep "expects a value" stderr &&

> +       test_must_fail git diff --raw --abbrev= HEAD~ 2>stderr &&
> +       test_i18ngrep "expects a value" stderr &&

Mismatch re i18n-ed or not between implementations and tests?

Martin