Web lists-archives.com

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

From: "Junio C Hamano" <gitster@xxxxxxxxx>
Sent: Wednesday, November 08, 2017 1:59 AM

"Philip Oakley" <philipoakley@xxxxxxx> writes:

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!

Hmmm, never thought about that.

I have been assuming that tools reading "--raw" output that is
abbreviated would be crazy, because they have to strip the dots and
the number of dots may not always be three [*1*].

But you are right.  It would be very unlikely that there is no such
crazy tools, so it deserves consideration if we would be breaking
such tools.

On the other hand, if such a crazy tool was still written correctly
(it is debatable what the definition of "correct" is, though), it
would be stripping any number dots at the end, not just insisting on
seeing exactly three dots, and splitting these fields at SP.
Otherwise they would already be broken as they cannot handle
occasional object names that have less than three dots because they
happen to be longer than the more common abbreviation length used by
other objects.  So in practice it might not be _too_ bad.

Thinking on this, I'd suggest that the patch series does remove the ellipsis dots immediately, but retains a config option that can be set to get back the old 'dots' display for those who have badly written scripts that maybe haven't failed yet. i.e. no deprecation period, just a fall back option; and if nobody shouts then remove the config option after a respectable period.

It would also mean the existing tests can be re-used...


*1* When we ask for --abbrev=7, we allocate 10 places and fill the
rest with necessary number of dots after the result of
find_unique_abbrev(), so if an object name turns out to require 8
hexdigits to make it unique, we'll append only two dots to it to
make it 10 so that it aligns nicely with others) and they would
always be reading the full, non abbreviated output.  The story does
not change that much when we do not explicitly ask for a specific
abbreviation length in that we add variable number of dots for
aligning in that case, too.

The --abbrev=7 does cater for many smaller repo's, so there is a possiblity that the bad script issue hasn't been hit yet by those repos.