Re: [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 19, 2017, 19:11 UTC
Message-ID: <CAPig+cT-r0uLLv_GyTRddPe=ATX883S1jt-8gc=ANZW21S81Mg@mail.gmail.com>
In-Reply-To: <20171119184113.16630-2-bedhanger@gmx.de>

On Sun, Nov 19, 2017 at 1:41 PM, Ann T Ropea <bedhanger@gmx.de> wrote:

Show 13 quoted lines

> Neither Git nor the user are in need of this (visual) aid anymore, but
> we must offer a transition period.
>
> Also, fix a typo: "abbbreviated" ---> "abbreviated".
>
> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---
> diff --git a/diff.c b/diff.c
> @@ -4901,15 +4901,22 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
>  {
>         int abblen;
>         const char *abbrev;
> +       const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS");

Can you move the getenv() call down to the point where the result is actually used so we don't have to worry about its value going stale[1] by some intervening call to getenv(), setenv(), unsetenv() or putenv()? Alternately, move the check against "yes" up here and assign it to a boolean (int) which you consult later.

Ditto for the other patches. Thanks.

[1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html

Show 11 quoted lines

> +       /* Do we want all 40 hex characters? */
>         if (len == GIT_SHA1_HEXSZ)
>                 return oid_to_hex(oid);
>
> +       /* An abbreviated value is fine, possibly followed by an ellipsis. */
>         abbrev = diff_abbrev_oid(oid, len);
> +
> +       if (!(env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes")))
> +               return abbrev;
> +
>         abblen = strlen(abbrev);