From: Philip Oakley Date: Thu, 09 Nov 2017 23:15:16 GMT Subject: Re: [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Message-ID: In-Reply-To: From: "Junio C Hamano" Sent: Wednesday, November 08, 2017 1:59 AM > "Philip Oakley" writes: > >> But... >>> ... >>> 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... > > [Footnote] > > *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. -- Philip