Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish

From: Philip Oakley <philipoakley@iee.org>
Date: Nov 7, 2017, 23:25 UTC
Message-ID: <83D263E58ABD46188756D41FE311E469@PhilipOakley>
In-Reply-To: <20171107025321.31028-1-bedhanger@gmx.de>

From: "Ann T Ropea" <bedhanger@gmx.de>

Show 8 quoted lines

> Thanks for all the feedback provided!
>
> I'd like to summarise what consensus we have reached so far and
> then propose a way forward:
>
>   * we'll use the term "ellipsis (pl. ellipses)" for what's
>     been referred to as "3dots", "n-dots", "many dots" and so
>     forth

Using a consistent  term for the *display* of shortened oid's is good.

>
>   * we would like to use ellipses when attached to SHA-1
>     values only for the purpose of specifying a symmetric
>     difference (as per gitrevisions(7))

The symetric difference (three-dots) is a specific Git *cli* notation that is distinct from the use of ellipsis for displaying oid's

>
>   * the usage of ellipses as a "here we truncated something
>     longer" is a relic which should be phased out.

I think that is true.

Show 5 quoted lines

>
> To get there, preventing describe_detached_head from appending
> an ellipsis to the SHA-1 values it prints is one important step.
>
> This change does not cause any test to fall over.

But...

Show 23 quoted lines

>
> The other important step is dealing with the "git diff --raw"
> output which features ellipses in the relic-fashion no longer
> desired.
>
> It would appear that simplifying diff.c's diff_aligned_abbrev
> routine to something like:
>
> /* Do we want all 40 hex characters?
> */
> if (len == GIT_SHA1_HEXSZ)
> return oid_to_hex(oid);
>
> /* An abbreviated value is fine.
> */
> return diff_abbrev_oid(oid, len);
>
> does do the trick.
>
> This change causes quite a few tests to fall over; however, they
> all have truncated-something-longer-ellipses in their
> raw-diff-output expected sections, and removing the ellipses
> from there makes the tests pass again, :-)

The number of failures you report in the test suit suggests that someone somewhere will be expecting that notation, and that we may need a deprecation period, perhaps with an 'ellipsis' config variable whose default value can later be flipped, though that leaves a config value needing support forever!

Junio should be able to better advise on his preferred approach.

>
> If we can agree that this is a way forward, i'll create & send
> v2 of the patch series to the mailing list (it'll include the
> fixed tests) and we'll see where we go from there.

-- Philip