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

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

[Ann T Ropea]

When a committish, C, is immediately followed by 3dots (...) which are,
in turn, not followed by another committish, we are usually asking for
the revision range C...HEAD, which is known as the symmetric difference
of C and HEAD.

When describe_detached_head is invoked, it prints the committish
followed by 3dots as elaborated above when it indicates the current and
previous HEAD positions.  For example:

   # Randomly check out one of the first seven git.git commits
   # (Starting with a detached HEAD already)
   $ git checkout $(git rev-list master | tail -7 | shuf | head -1)
   Previous HEAD position was bfcf2d7874f7... checkout: describe_detached_head: remove 3dots after committish
   HEAD is now at 19b2860cba57... Use "-Wall -O2" for the compiler to get more warnings.

"Evaluating" this displayed pseudo-range for the current HEAD indication
resolves to the empty range (C...HEAD, where C equals HEAD).

For the previous HEAD indication, the results of the "evaluation" are
somewhat more difficult to predict: previous here refers to what the
reflog dictates (this is not necessarily the topological ancestor in the
DAG, i.e., HEAD^).  In the example above, the "range" resolves to almost
all commits in the author's clone of git.git.  Running the command again
causes the then previous to current HEAD position "range" to be a lot
smaller.

This could be confusing not only for novices; in either case, no range
should be insinuated by describe_detached_head.

Granted, this "evaluation" is at the moment, if at all, only performed
in the mind of the observer.  And, to be sure, the 3dots *are* intended
as a continuation indication for the abbreviated SHA-1 value.
Nevertheless, we should get rid of them, for the following reasons:

   * they would still be displayed if someone had their core.abbrev
     config value set to the max

   * when the built-in version of checkout was introduced by commit

        782c2d65c240 ("Build in checkout", 2008-02-07)

     no 3dots were present in the legacy git-checkout.sh (see
     contrib/examples/git-checkout.sh)

   * when git-reset causes a new HEAD line to be printed (during a hard
     reset), neither builtin/reset.c nor contrib/examples/git-reset.sh
     mention 3dots

Lest we confuse the meticulous observer, we ought to retire the 3dots in
the circumstances described above.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
 builtin/checkout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea29..59cc52e55855 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -404,7 +404,7 @@ static void describe_detached_head(const char *msg, struct commit *commit)
 	struct strbuf sb = STRBUF_INIT;
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
+	fprintf(stderr, "%s %s %s\n", msg,
 		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
 	strbuf_release(&sb);
 }
-- 
2.13.6

[PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots)

[Ann T Ropea]

Using them as continuation indications for abbreviated SHA-1 values
bears the risk that they are mistaken for the dotted range operators.

Commands which expect a single commit will fail when given a range of
commits.

Also, add a note that sometimes, 3dots are just continuation
indications.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
 Documentation/user-manual.txt | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index b4d88af133e8..bdb44b067399 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
@@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this
 
 If you run `git branch` at this point, you'll see that Git has
 temporarily moved you in "(no branch)". HEAD is now detached from any
-branch and points directly to a commit (with commit id 65934...) that
+branch and points directly to a commit (with commit id 65934) that
 is reachable from "master" but not from v2.6.18. Compile and test it,
 and see whether it crashes. Assume it does crash. Then:
 
@@ -549,14 +549,14 @@ says "bisect".  Choose a safe-looking commit nearby, note its commit
 id, and check it out with:
 
 -------------------------------------------------
-$ git reset --hard fb47ddb2db...
+$ git reset --hard fb47ddb2db
 -------------------------------------------------
 
 then test, run `bisect good` or `bisect bad` as appropriate, and
 continue.
 
 Instead of `git bisect visualize` and then `git reset --hard
-fb47ddb2db...`, you might just want to tell Git that you want to skip
+fb47ddb2db`, you might just want to tell Git that you want to skip
 the current commit:
 
 -------------------------------------------------
@@ -3426,6 +3426,8 @@ Date:
 ...
 :100644 100644 oldsha... 4b9458b... M somedirectory/myfile
 ------------------------------------------------
+(Note that in the above, the "..." are used as continuation
+indications, not as symmetric difference operators!)
 
 This tells you that the immediately following version of the file was
 "newsha", and that the immediately preceding version was "oldsha".
@@ -3449,7 +3451,7 @@ and your repository is good again!
 $ git log --raw --all
 ------------------------------------------------
 
-and just looked for the sha of the missing object (4b9458b..) in that
+and just looked for the sha of the missing object (4b9458b) in that
 whole thing. It's up to you--Git does *have* a lot of information, it is
 just missing one particular blob version.
 
@@ -4114,9 +4116,9 @@ program, e.g.  `diff3`, `merge`, or Git's own merge-file, on
 the blob objects from these three stages yourself, like this:
 
 ------------------------------------------------
-$ git cat-file blob 263414f... >hello.c~1
-$ git cat-file blob 06fa6a2... >hello.c~2
-$ git cat-file blob cc44c73... >hello.c~3
+$ git cat-file blob 263414f >hello.c~1
+$ git cat-file blob 06fa6a2 >hello.c~2
+$ git cat-file blob cc44c73 >hello.c~3
 $ git merge-file hello.c~2 hello.c~1 hello.c~3
 ------------------------------------------------
 
@@ -4374,7 +4376,7 @@ $ git log --no-merges t/
 ------------------------
 
 In the pager (`less`), just search for "bundle", go a few lines back,
-and see that it is in commit 18449ab0...  Now just copy this object name,
+and see that it is in commit 18449ab0.  Now just copy this object name,
 and paste it into the command line
 
 -------------------
-- 
2.13.6

[PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications

[Ann T Ropea]

Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot").

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
 Documentation/revisions.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..d1b126427177 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
@@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the origin do since
 I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
 empty range that is both reachable and unreachable from HEAD.
 
+However, there are instances where '<sha1>...' is *not*
+equivalent to '<sha1>...HEAD'.  See the "RAW OUTPUT FORMAT"
+section of linkgit:git-diff[1]: the three-dot notations used
+there are simply continuation indications for the abbreviated
+SHA-1 values.  The ones encountered there are usually
+associated with file/index/tree contents rather than with commit
+objects, and the range operators described above are only
+applicable to commit objects (i.e., 'r1' and 'r2').
+
 Other <rev>{caret} Parent Shorthand Notations
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Three other shorthands exist, particularly useful for merge commits,
-- 
2.13.6

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

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> This could be confusing not only for novices; in either case, no range
> should be insinuated by describe_detached_head.

We actually do not insinuate any range in these output.  These dots
denote "truncated at the end, instead of giving full length."

Another place these "many dots" appear is "git diff --raw", for
example.

Re: [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> Also, fix typo: "three dot" ---> "three-dot" (align with "two-dot").
>
> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---
>  Documentation/revisions.txt | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index 61277469c874..d1b126427177 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
>   for commits that are reachable from r2 excluding those that are reachable
>   from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
>  
> -The '...' (three dot) Symmetric Difference Notation::
> +The '...' (three-dot) Symmetric Difference Notation::
>   A similar notation 'r1\...r2' is called symmetric difference
>   of 'r1' and 'r2' and is defined as
>   'r1 r2 --not $(git merge-base --all r1 r2)'.
> @@ -285,6 +285,15 @@ is a shorthand for 'HEAD..origin' and asks "What did the origin do since
>  I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
>  empty range that is both reachable and unreachable from HEAD.
>  
> +However, there are instances where '<sha1>...' is *not*
> +equivalent to '<sha1>...HEAD'.  See the "RAW OUTPUT FORMAT"
> +section of linkgit:git-diff[1]: the three-dot notations used
> +there are simply continuation indications for the abbreviated
> +SHA-1 values.  The ones encountered there are usually
> +associated with file/index/tree contents rather than with commit
> +objects, and the range operators described above are only
> +applicable to commit objects (i.e., 'r1' and 'r2').
> +
>  Other <rev>{caret} Parent Shorthand Notations
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  Three other shorthands exist, particularly useful for merge commits,

I actually have a mild suspicion that this is going in a wrong
direction.  In very early days of Git, we wanted to make sure that
people can tell if a long hex string is a truncated object name or a
full one (mostly because some lower-level commands insisted to be
fed only full object name).

These days, everybody knows when they see 79ec0be62a that it is
*not* a full object name and will no longer be confused unlike early
days and there is no strong reason to waste six output columns of
"git diff --raw" output by using these three dots.  

I wonder if we can come up with a solution in line with the patch
1/3 in this series, which got rid of the "..." that indicated that
the hexstring was not a full object name.

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

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Ann T Ropea <bedhanger@gmx.de> writes:
>
>> This could be confusing not only for novices; in either case, no range
>> should be insinuated by describe_detached_head.
>
> We actually do not insinuate any range in these output.  These dots
> denote "truncated at the end, instead of giving full length."
>
> Another place these "many dots" appear is "git diff --raw", for
> example.
>

<bikeshed> The fancy word for the three dots is an `ellipsis`
- the omission from speech or writing of a word or words that are 
superfluous or able to be understood from contextual clues.
- from the Ancient Greek: ἔλλειψις, élleipsis, "omission" or "falling 
short".

The user/reader confusion may still be there though.
</bikeshed> 

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

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> Lest we confuse the meticulous observer, we ought to retire the 3dots in
> the circumstances described above.

Yes, as I said in my response to 3/3, I think it is a good goal to
avoid n-dots used as a "here we truncated something longer" sign,
which was a very old convention that was invented without knowing
that we'd later come up with a syntax that would conflict with it.

For this particular output, I wonder if it is even better to follow
our own advice, though.  Documentation/SubmittingPatches says:

    If you want to reference a previous commit in the history of a stable
    branch, use the format "abbreviated sha1 (subject, date)",
    with the subject enclosed in a pair of double-quotes, like this:

        Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
        noticed that ...

I dunno.

> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---
>  builtin/checkout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index fc4f8fd2ea29..59cc52e55855 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -404,7 +404,7 @@ static void describe_detached_head(const char *msg, struct commit *commit)
>  	struct strbuf sb = STRBUF_INIT;
>  	if (!parse_commit(commit))
>  		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
> -	fprintf(stderr, "%s %s... %s\n", msg,
> +	fprintf(stderr, "%s %s %s\n", msg,
>  		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
>  	strbuf_release(&sb);
>  }

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

[Ann T Ropea]

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

   * 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 usage of ellipses as a "here we truncated something
     longer" is a relic which should be phased out

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.

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, :-)

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.

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

[Philip Oakley]

From: "Ann T Ropea" <bedhanger@gmx.de>
> 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.

>
> 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...
>
> 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 

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

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> 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.


[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.

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

[Philip Oakley]

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


> "Philip Oakley" <philipoakley@iee.org> 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 

Re: [PATCH v2 1/6] config: introduce core.printsha1ellipsis

[Ann T Ropea]

We use it to ascertain whether or not the user wants an ellipsis
printed after an abbreviated SHA-1 value.

By de-asserting it in the default, we encourage not printing the
ellipsis in such circumstances.

Not adding a corresponding cli option is intentional; and we would like
to remove the config option *and* the ellipses after abbreviated SHA-1
values after a respectable period.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
 Documentation/config.txt | 10 ++++++++++
 cache.h                  |  1 +
 config.c                 |  9 +++++++++
 environment.c            |  1 +
 4 files changed, 21 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 671fcbaa0fd1..93887820ff89 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -898,6 +898,16 @@ core.abbrev::
 	abbreviated object names to stay unique for some time.
 	The minimum length is 4.
 
+core.printsha1ellipsis (deprecated)::
+	A Boolean controlling whether an ellipsis should be
+	printed following an (abbreviated) SHA-1 value.  This
+	affects indications of detached HEADs or the raw diff
+	output.  Printing an ellipsis in the cases mentioned is no
+	longer considered adequate and support for it will be
+	removed in the foreseeable future (along with the
+	option).
+	Therefore, the default is false.
+
 add.ignoreErrors::
 add.ignore-errors (deprecated)::
 	Tells 'git add' to continue adding files when some files cannot be
diff --git a/cache.h b/cache.h
index cb7fb7c004be..5cbc50a0c1ab 100644
--- a/cache.h
+++ b/cache.h
@@ -753,6 +753,7 @@ extern int check_stat;
 extern int quote_path_fully;
 extern int has_symlinks;
 extern int minimum_abbrev, default_abbrev;
+extern int print_sha1_ellipsis;
 extern int ignore_case;
 extern int assume_unchanged;
 extern int prefer_symlink_refs;
diff --git a/config.c b/config.c
index 903abf9533b1..f560aea5e98b 100644
--- a/config.c
+++ b/config.c
@@ -1073,6 +1073,15 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	/* Printing an ellipsis after an abbreviated SHA-1 value
+	 * is no longer desired but must be selectable for some
+	 * time to come.
+	 */
+	if (!strcmp(var, "core.printsha1ellipsis")) {
+		print_sha1_ellipsis = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.disambiguate"))
 		return set_disambiguate_hint_config(var, value);
 
diff --git a/environment.c b/environment.c
index 8289c25b44d7..5ceb92f921c6 100644
--- a/environment.c
+++ b/environment.c
@@ -19,6 +19,7 @@ int trust_ctime = 1;
 int check_stat = 1;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = -1;
+int print_sha1_ellipsis = 0; /* Only if the user requests it. */
 int ignore_case;
 int assume_unchanged;
 int prefer_symlink_refs;
-- 
2.13.6

Re: [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user (actively!) sets the config option core.printsha1ellipsis to
true.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
 builtin/checkout.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6c2b4cd419a4..101a16a14a76 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -402,8 +402,13 @@ static void describe_detached_head(const char *msg, struct commit *commit)
 	struct strbuf sb = STRBUF_INIT;
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (print_sha1_ellipsis) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
-- 
2.13.6

Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Ann T Ropea]

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>
---
v2: rename patch series & focus on removal of ellipses
 diff.c | 69 +++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/diff.c b/diff.c
index 0763e89263ef..9709dc37c6d7 100644
--- a/diff.c
+++ b/diff.c
@@ -4902,41 +4902,50 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
 	int abblen;
 	const char *abbrev;
 
+	/* Do we want all 40 hex characters?
+	 */
 	if (len == GIT_SHA1_HEXSZ)
 		return oid_to_hex(oid);
 
-	abbrev = diff_abbrev_oid(oid, len);
-	abblen = strlen(abbrev);
-
-	/*
-	 * In well-behaved cases, where the abbbreviated result is the
-	 * same as the requested length, append three dots after the
-	 * abbreviation (hence the whole logic is limited to the case
-	 * where abblen < 37); when the actual abbreviated result is a
-	 * bit longer than the requested length, we reduce the number
-	 * of dots so that they match the well-behaved ones.  However,
-	 * if the actual abbreviation is longer than the requested
-	 * length by more than three, we give up on aligning, and add
-	 * three dots anyway, to indicate that the output is not the
-	 * full object name.  Yes, this may be suboptimal, but this
-	 * appears only in "diff --raw --abbrev" output and it is not
-	 * worth the effort to change it now.  Note that this would
-	 * likely to work fine when the automatic sizing of default
-	 * abbreviation length is used--we would be fed -1 in "len" in
-	 * that case, and will end up always appending three-dots, but
-	 * the automatic sizing is supposed to give abblen that ensures
-	 * uniqueness across all objects (statistically speaking).
+	/* An abbreviated value is fine, possibly followed by an
+	 * ellipsis.
 	 */
-	if (abblen < GIT_SHA1_HEXSZ - 3) {
-		static char hex[GIT_MAX_HEXSZ + 1];
-		if (len < abblen && abblen <= len + 2)
-			xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
-		else
-			xsnprintf(hex, sizeof(hex), "%s...", abbrev);
-		return hex;
-	}
+	if (print_sha1_ellipsis) {
+		abbrev = diff_abbrev_oid(oid, len);
+		abblen = strlen(abbrev);
+
+		/*
+		 * In well-behaved cases, where the abbreviated result is the
+		 * same as the requested length, append three dots after the
+		 * abbreviation (hence the whole logic is limited to the case
+		 * where abblen < 37); when the actual abbreviated result is a
+		 * bit longer than the requested length, we reduce the number
+		 * of dots so that they match the well-behaved ones.  However,
+		 * if the actual abbreviation is longer than the requested
+		 * length by more than three, we give up on aligning, and add
+		 * three dots anyway, to indicate that the output is not the
+		 * full object name.  Yes, this may be suboptimal, but this
+		 * appears only in "diff --raw --abbrev" output and it is not
+		 * worth the effort to change it now.  Note that this would
+		 * likely to work fine when the automatic sizing of default
+		 * abbreviation length is used--we would be fed -1 in "len" in
+		 * that case, and will end up always appending three-dots, but
+		 * the automatic sizing is supposed to give abblen that ensures
+		 * uniqueness across all objects (statistically speaking).
+		 */
+		if (abblen < GIT_SHA1_HEXSZ - 3) {
+			static char hex[GIT_MAX_HEXSZ + 1];
+			if (len < abblen && abblen <= len + 2)
+				xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
+			else
+				xsnprintf(hex, sizeof(hex), "%s...", abbrev);
+			return hex;
+		}
 
-	return oid_to_hex(oid);
+		return oid_to_hex(oid);
+	} else {
+		return diff_abbrev_oid(oid, len);
+	}
 }
 
 static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
-- 
2.13.6

Re: [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis

[Ann T Ropea]

Confusing the ellipsis with the three-dot operator should be made as
difficult as possible.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
 Documentation/user-manual.txt | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 3a03e63eb0d8..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
@@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this
 
 If you run `git branch` at this point, you'll see that Git has
 temporarily moved you in "(no branch)". HEAD is now detached from any
-branch and points directly to a commit (with commit id 65934...) that
+branch and points directly to a commit (with commit id 65934) that
 is reachable from "master" but not from v2.6.18. Compile and test it,
 and see whether it crashes. Assume it does crash. Then:
 
@@ -549,14 +549,14 @@ says "bisect".  Choose a safe-looking commit nearby, note its commit
 id, and check it out with:
 
 -------------------------------------------------
-$ git reset --hard fb47ddb2db...
+$ git reset --hard fb47ddb2db
 -------------------------------------------------
 
 then test, run `bisect good` or `bisect bad` as appropriate, and
 continue.
 
 Instead of `git bisect visualize` and then `git reset --hard
-fb47ddb2db...`, you might just want to tell Git that you want to skip
+fb47ddb2db`, you might just want to tell Git that you want to skip
 the current commit:
 
 -------------------------------------------------
@@ -3416,7 +3416,7 @@ commit abc
 Author:
 Date:
 ...
-:100644 100644 4b9458b... newsha... M somedirectory/myfile
+:100644 100644 4b9458b newsha M somedirectory/myfile
 
 
 commit xyz
@@ -3424,7 +3424,7 @@ Author:
 Date:
 
 ...
-:100644 100644 oldsha... 4b9458b... M somedirectory/myfile
+:100644 100644 oldsha 4b9458b M somedirectory/myfile
 ------------------------------------------------
 
 This tells you that the immediately following version of the file was
@@ -3449,7 +3449,7 @@ and your repository is good again!
 $ git log --raw --all
 ------------------------------------------------
 
-and just looked for the sha of the missing object (4b9458b..) in that
+and just looked for the sha of the missing object (4b9458b) in that
 whole thing. It's up to you--Git does *have* a lot of information, it is
 just missing one particular blob version.
 
@@ -4114,9 +4114,9 @@ program, e.g.  `diff3`, `merge`, or Git's own merge-file, on
 the blob objects from these three stages yourself, like this:
 
 ------------------------------------------------
-$ git cat-file blob 263414f... >hello.c~1
-$ git cat-file blob 06fa6a2... >hello.c~2
-$ git cat-file blob cc44c73... >hello.c~3
+$ git cat-file blob 263414f >hello.c~1
+$ git cat-file blob 06fa6a2 >hello.c~2
+$ git cat-file blob cc44c73 >hello.c~3
 $ git merge-file hello.c~2 hello.c~1 hello.c~3
 ------------------------------------------------
 
@@ -4374,7 +4374,7 @@ $ git log --no-merges t/
 ------------------------
 
 In the pager (`less`), just search for "bundle", go a few lines back,
-and see that it is in commit 18449ab0...  Now just copy this object name,
+and see that it is in commit 18449ab0.  Now just copy this object name,
 and paste it into the command line
 
 -------------------
-- 
2.13.6

Re: [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot").

[Ann T Ropea]

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..dfcc49c72c0f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
-- 
2.13.6

Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values

[Ann T Ropea]

Where needed, we arrange for invocations of Git as if

   "-c core.printsha1ellipsis=true"

had been specified on the command-line.  This furnishes ellipses in the
output which then matches what is expected.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
 t/t3040-subprojects-basic.sh | 12 ++++++++++++
 t/t4013-diff-various.sh      | 12 ++++++++++++
 t/t9300-fast-import.sh       | 12 ++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
index 0a4ff6d824a0..63b85bfdd4f9 100755
--- a/t/t3040-subprojects-basic.sh
+++ b/t/t3040-subprojects-basic.sh
@@ -3,6 +3,18 @@
 test_description='Basic subproject functionality'
 . ./test-lib.sh
 
+# Some of the tests expect an ellipsis after the (abbreviated)
+# SHA-1 value.  The code below results in Git being called with
+# "-c core.printsha1ellipsis=true" which satisfies those tests.
+do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
+if test -z "${GIT_CONFIG_PARAMETERS}"
+then
+	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
+else
+	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
+fi
+export GIT_CONFIG_PARAMETERS
+
 test_expect_success 'setup: create superproject' '
 	: >Makefile &&
 	git add Makefile &&
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index c515e3e53fee..8ee14c7c6796 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -7,6 +7,18 @@ test_description='Various diff formatting options'
 
 . ./test-lib.sh
 
+# Some of the tests expect an ellipsis after the (abbreviated)
+# SHA-1 value.  The code below results in Git being called with
+# "-c core.printsha1ellipsis=true" which satisfies those tests.
+do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
+if test -z "${GIT_CONFIG_PARAMETERS}"
+then
+	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
+else
+	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
+fi
+export GIT_CONFIG_PARAMETERS
+
 LF='
 '
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d47560b6343d..6cc41b90dafa 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -7,6 +7,18 @@ test_description='test git fast-import utility'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
 
+# Some of the tests expect an ellipsis after the (abbreviated)
+# SHA-1 value.  The code below results in Git being called with
+# "-c core.printsha1ellipsis=true" which satisfies those tests.
+do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
+if test -z "${GIT_CONFIG_PARAMETERS}"
+then
+	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
+else
+	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
+fi
+export GIT_CONFIG_PARAMETERS
+
 verify_packs () {
 	for p in .git/objects/pack/*.pack
 	do
-- 
2.13.6

Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> 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>
> ---
> v2: rename patch series & focus on removal of ellipses
>  diff.c | 69 +++++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 0763e89263ef..9709dc37c6d7 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4902,41 +4902,50 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
>  	int abblen;
>  	const char *abbrev;
>  
> +	/* Do we want all 40 hex characters?
> +	 */

The oldest parts of the codebase (including diff.c) deviate from the
rule in places, but in our multi-line comment, /* and */ occupy a
line on their own.  In this case, a single-line comment should be
sufficient, though.

>  	if (len == GIT_SHA1_HEXSZ)
>  		return oid_to_hex(oid);
>  
> -	abbrev = diff_abbrev_oid(oid, len);
> -	abblen = strlen(abbrev);
> -
> -	/*
> -	 * In well-behaved cases, where the abbbreviated result is the
> -	 * same as the requested length, append three dots after the
> -	 * abbreviation (hence the whole logic is limited to the case
> -	 * where abblen < 37); when the actual abbreviated result is a
> -	 * bit longer than the requested length, we reduce the number
> -	 * of dots so that they match the well-behaved ones.  However,
> -	 * if the actual abbreviation is longer than the requested
> -	 * length by more than three, we give up on aligning, and add
> -	 * three dots anyway, to indicate that the output is not the
> -	 * full object name.  Yes, this may be suboptimal, but this
> -	 * appears only in "diff --raw --abbrev" output and it is not
> -	 * worth the effort to change it now.  Note that this would
> -	 * likely to work fine when the automatic sizing of default
> -	 * abbreviation length is used--we would be fed -1 in "len" in
> -	 * that case, and will end up always appending three-dots, but
> -	 * the automatic sizing is supposed to give abblen that ensures
> -	 * uniqueness across all objects (statistically speaking).
> +	/* An abbreviated value is fine, possibly followed by an
> +	 * ellipsis.
>  	 */

Ditto.

> -	if (abblen < GIT_SHA1_HEXSZ - 3) {
> -		static char hex[GIT_MAX_HEXSZ + 1];
> -		if (len < abblen && abblen <= len + 2)
> -			xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
> -		else
> -			xsnprintf(hex, sizeof(hex), "%s...", abbrev);
> -		return hex;
> -	}
> +	if (print_sha1_ellipsis) {
> +		abbrev = diff_abbrev_oid(oid, len);
> +		abblen = strlen(abbrev);
> +
> +		/*
> +		 * In well-behaved cases, where the abbreviated result is the
> +		 * same as the requested length, append three dots after the
> +		 * abbreviation (hence the whole logic is limited to the case
> +		 * where abblen < 37); when the actual abbreviated result is a
> +		 * bit longer than the requested length, we reduce the number
> +		 * of dots so that they match the well-behaved ones.  However,
> +		 * if the actual abbreviation is longer than the requested
> +		 * length by more than three, we give up on aligning, and add
> +		 * three dots anyway, to indicate that the output is not the
> +		 * full object name.  Yes, this may be suboptimal, but this
> +		 * appears only in "diff --raw --abbrev" output and it is not
> +		 * worth the effort to change it now.  Note that this would
> +		 * likely to work fine when the automatic sizing of default
> +		 * abbreviation length is used--we would be fed -1 in "len" in
> +		 * that case, and will end up always appending three-dots, but
> +		 * the automatic sizing is supposed to give abblen that ensures
> +		 * uniqueness across all objects (statistically speaking).
> +		 */
> +		if (abblen < GIT_SHA1_HEXSZ - 3) {
> +			static char hex[GIT_MAX_HEXSZ + 1];
> +			if (len < abblen && abblen <= len + 2)
> +				xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
> +			else
> +				xsnprintf(hex, sizeof(hex), "%s...", abbrev);
> +			return hex;
> +		}
>  
> -	return oid_to_hex(oid);
> +		return oid_to_hex(oid);
> +	} else {
> +		return diff_abbrev_oid(oid, len);
> +	}
>  }

If I were writing this, I would have called diff_abbrev_oid() before
checking print_sha1_ellipsis, returned it if it is not set.  That
way the indentation level of the code would not have to change.

HOWEVER.

Notice the name of the function.  We no longer even attempt to align
the output, and in general the output column length of each line
would be shorter than the original.  I am wondering if the change
would be of less impact if we try to abbreviate to len+3 and then
chomp the result at the right hand side to len+3 (only if the result
is unique) when print_sha1_ellipsis is false.  Of course, once we go
that path, the code structure this patch introduces (not the one I
mentioned in the previous paragraph) would be necessary.  Essentially
you would be enhancing the "else" clause.


>  
>  static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)

Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> Where needed, we arrange for invocations of Git as if
>
>    "-c core.printsha1ellipsis=true"
>
> had been specified on the command-line.  This furnishes ellipses in the
> output which then matches what is expected.
>
> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---

I am not a huge fan of exposing undocumented implementation details
to the test scripts that much.  There are three in t13xx series that
mention GIT_CONFIG_PARAMETERS, but these are about testing the config
mechanism itself.  

An end-user script would instead be doing "git -c var=val" for each
invocation but this one is being lazy because it does not want to
bother identifying which "git" invocation needs the treatment and
also it does not want to keep maintaining it, which is understandable,
but it feels dirty.

This makes me wonder if core.printsha1ellipsis should really be a
configuration variable in the first place.  Wouldn't an environment
variable be more appropriate?  After all, the users of scripts that
would be broken by this series would need to the same thing as what
we do to our tests to keep them working while they update them.

> v2: rename patch series & focus on removal of ellipses
>  t/t3040-subprojects-basic.sh | 12 ++++++++++++
>  t/t4013-diff-various.sh      | 12 ++++++++++++
>  t/t9300-fast-import.sh       | 12 ++++++++++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
> index 0a4ff6d824a0..63b85bfdd4f9 100755
> --- a/t/t3040-subprojects-basic.sh
> +++ b/t/t3040-subprojects-basic.sh
> @@ -3,6 +3,18 @@
>  test_description='Basic subproject functionality'
>  . ./test-lib.sh
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  test_expect_success 'setup: create superproject' '
>  	: >Makefile &&
>  	git add Makefile &&
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index c515e3e53fee..8ee14c7c6796 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -7,6 +7,18 @@ test_description='Various diff formatting options'
>  
>  . ./test-lib.sh
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  LF='
>  '
>  
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index d47560b6343d..6cc41b90dafa 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -7,6 +7,18 @@ test_description='test git fast-import utility'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  verify_packs () {
>  	for p in .git/objects/pack/*.pack
>  	do

Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Ann T Ropea]

Junio C Hamano <gitster@pobox.com> writes:

> HOWEVER.

> Notice the name of the function.  We no longer even attempt to align
> the output, and in general the output column length of each line
> would be shorter than the original.  I am wondering if the change
> would be of less impact if we try to abbreviate to len+3 and then
> chomp the result at the right hand side to len+3 (only if the result
> is unique) when print_sha1_ellipsis is false.  Of course, once we go
> that path, the code structure this patch introduces (not the one I
> mentioned in the previous paragraph) would be necessary.  Essentially
> you would be enhancing the "else" clause.

Sorry, but you've lost me there.

I'm in the process of producing v3 of the series (env var
instead of config, comments, indentation level), but i can't get
my head around the above.

Would you care to elaborate (then based on v3)?

Re: [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user (actively!) sets the environment variable PRINT_SHA1_ELLIPSIS
to "yes" (case does not matter).

The transient nature of this fallback suggests that we should not prefix
the variable by "GIT_".

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
 builtin/checkout.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7d8bcc383351..e6d3a28fe26e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,10 +400,17 @@ static void show_local_changes(struct object *head,
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+	const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS");
+
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes")) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
-- 
2.13.6

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

[Ann T Ropea]

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>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
 diff.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 0763e89263ef..359c6fc85a27 100644
--- 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");
 
+	/* 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);
 
 	/*
-	 * In well-behaved cases, where the abbbreviated result is the
+	 * In well-behaved cases, where the abbreviated result is the
 	 * same as the requested length, append three dots after the
 	 * abbreviation (hence the whole logic is limited to the case
 	 * where abblen < 37); when the actual abbreviated result is a
-- 
2.13.6

Re: [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis

[Ann T Ropea]

Confusing the ellipsis with the three-dot operator should be made as
difficult as possible.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
 Documentation/user-manual.txt | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 3a03e63eb0d8..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
@@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this
 
 If you run `git branch` at this point, you'll see that Git has
 temporarily moved you in "(no branch)". HEAD is now detached from any
-branch and points directly to a commit (with commit id 65934...) that
+branch and points directly to a commit (with commit id 65934) that
 is reachable from "master" but not from v2.6.18. Compile and test it,
 and see whether it crashes. Assume it does crash. Then:
 
@@ -549,14 +549,14 @@ says "bisect".  Choose a safe-looking commit nearby, note its commit
 id, and check it out with:
 
 -------------------------------------------------
-$ git reset --hard fb47ddb2db...
+$ git reset --hard fb47ddb2db
 -------------------------------------------------
 
 then test, run `bisect good` or `bisect bad` as appropriate, and
 continue.
 
 Instead of `git bisect visualize` and then `git reset --hard
-fb47ddb2db...`, you might just want to tell Git that you want to skip
+fb47ddb2db`, you might just want to tell Git that you want to skip
 the current commit:
 
 -------------------------------------------------
@@ -3416,7 +3416,7 @@ commit abc
 Author:
 Date:
 ...
-:100644 100644 4b9458b... newsha... M somedirectory/myfile
+:100644 100644 4b9458b newsha M somedirectory/myfile
 
 
 commit xyz
@@ -3424,7 +3424,7 @@ Author:
 Date:
 
 ...
-:100644 100644 oldsha... 4b9458b... M somedirectory/myfile
+:100644 100644 oldsha 4b9458b M somedirectory/myfile
 ------------------------------------------------
 
 This tells you that the immediately following version of the file was
@@ -3449,7 +3449,7 @@ and your repository is good again!
 $ git log --raw --all
 ------------------------------------------------
 
-and just looked for the sha of the missing object (4b9458b..) in that
+and just looked for the sha of the missing object (4b9458b) in that
 whole thing. It's up to you--Git does *have* a lot of information, it is
 just missing one particular blob version.
 
@@ -4114,9 +4114,9 @@ program, e.g.  `diff3`, `merge`, or Git's own merge-file, on
 the blob objects from these three stages yourself, like this:
 
 ------------------------------------------------
-$ git cat-file blob 263414f... >hello.c~1
-$ git cat-file blob 06fa6a2... >hello.c~2
-$ git cat-file blob cc44c73... >hello.c~3
+$ git cat-file blob 263414f >hello.c~1
+$ git cat-file blob 06fa6a2 >hello.c~2
+$ git cat-file blob cc44c73 >hello.c~3
 $ git merge-file hello.c~2 hello.c~1 hello.c~3
 ------------------------------------------------
 
@@ -4374,7 +4374,7 @@ $ git log --no-merges t/
 ------------------------
 
 In the pager (`less`), just search for "bundle", go a few lines back,
-and see that it is in commit 18449ab0...  Now just copy this object name,
+and see that it is in commit 18449ab0.  Now just copy this object name,
 and paste it into the command line
 
 -------------------
-- 
2.13.6

Re: [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot").

[Ann T Ropea]

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..dfcc49c72c0f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
-- 
2.13.6

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Ann T Ropea]

Where needed[*1*], we arrange for the environment of a Git invocation to
contain the variable

   PRINT_SHA1_ELLIPSIS="yes"

This furnishes ellipses in the output which then matches what is
expected.

[Footnote]

*1* We are being overly generous in t4013-diff-various.sh because we do
not want to destroy/take apart the here-document.  Given that all this a
temporary measure, we should get away with it.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
 t/t3040-subprojects-basic.sh | 2 +-
 t/t4013-diff-various.sh      | 6 +++++-
 t/t9300-fast-import.sh       | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
index 0a4ff6d824a0..dfd46a5672da 100755
--- a/t/t3040-subprojects-basic.sh
+++ b/t/t3040-subprojects-basic.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' '
 	git update-index --add sub1 &&
 	git add sub2 &&
 	git commit -q -m "subprojects added" &&
-	git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
+	PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
 	git branch save HEAD &&
 	cat >expected <<-\EOF &&
 	:000000 160000 00000... A	sub1
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index c515e3e53fee..79f213dddd92 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -131,7 +131,11 @@ do
 	test_expect_success "git $cmd" '
 		{
 			echo "\$ git $cmd"
-			git $cmd |
+			# All invocations receive the env var.
+			# We would destroy the here-doc if we
+			# tried to apply it to only the ones
+			# truly needing it.
+			PRINT_SHA1_ELLIPSIS="yes" git $cmd |
 			sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \
 			    -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/"
 			echo "\$"
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d47560b6343d..d93ddd91093e 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' '
 	EXPECT_END
 
 	git fast-import <input &&
-	git diff-tree --abbrev --raw L^ L >output &&
+	PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output &&
 	test_cmp expect output
 '
 
-- 
2.13.6

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

[Eric Sunshine]

On Sun, Nov 19, 2017 at 1:41 PM, Ann T Ropea <bedhanger@gmx.de> wrote:
> 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

> +       /* 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);

Re: [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis

[Eric Sunshine]

On Sun, Nov 19, 2017 at 1:41 PM, Ann T Ropea <bedhanger@gmx.de> wrote:
> Confusing the ellipsis with the three-dot operator should be made as
> difficult as possible.
>
> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---
> v2: rename patch series & focus on removal of ellipses
> v3: env var instead of config option, use one-line comments where appropriate, preserve indent level

I dont' see the  new PRINT_SHA1_ELLIPSIS environment variable
documented anywhere in this series. Did I overlook it? Without
documentation, it's not very discoverable (aside from reading the
source code).

Re: [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

>> Notice the name of the function.  We no longer even attempt to align
>> the output, and in general the output column length of each line
>> would be shorter than the original.  I am wondering if the change
>> would be of less impact if we try to abbreviate to len+3 and then
>> chomp the result at the right hand side to len+3 (only if the result
>> is unique) when print_sha1_ellipsis is false.  Of course, once we go
>> that path, the code structure this patch introduces (not the one I
>> mentioned in the previous paragraph) would be necessary.  Essentially
>> you would be enhancing the "else" clause.
>
> Sorry, but you've lost me there.

After diff_aligned_abbrev(const struct object_id *oid, int len)
returns the full-size result when asked, it does this:

	abbrev = diff_abbrev_oid(oid, len);
	abblen = strlen(abbrev);

	if (abblen < GIT_SHA1_HEXSZ - 3) {
		static char hex[GIT_MAX_HEXSZ + 1];
		if (len < abblen && abblen <= len + 2)
			xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
		else
			xsnprintf(hex, sizeof(hex), "%s...", abbrev);
		return hex;
	}

When asked to abbreviate to 4, which is shorter than 40-3=37, and
the unique abbreviation results in "dead" (iow, no other object that
shares that prefix), then len=4, abblen=4, so we get "dead..." as
the output.  If the unique abbreviation is "deadb" (iow, some other
object shares "dead" prefix and we require one more hexdigit for
uniqueness), len=4, abblen=5, and abblen <= len + 2, so we get
"deadb.." as the output.  Either codepath yields 7 hexdigits, and
this is because the function wants to help the user of its output
to produce lines that are as aligned as possible.

Of course, the leeway we have ... that can be reduced is small and
limited.  For a len that is too small in a repository with many
objects that share the same prefix, we cannot fit the result in 7
(i.e. len+3) columns and may have to return a longer result.

But the point is that the existing callers that ask for len=7 are
expecting output that is "7+3=10 output columns most of the time,
even though it might yield a string longer than 10 output columns
when the object names cannot be uniquely shown with that output
width".  

And that was why I was wondering if it gives less negative impact to
callers if we abbreviate to (len+3) hexdigits and return the result.

Note that this was not (and is not) "I think yours is wrong
because..." but was (and is) "I am wondering if doing it differently
is better because...".  Asking for 7 and getting 10 feels strange
and that is one valid reason to "correct" the existing behaviour by
abbreviating to len and just dropping the padding with dots, and it
may be worth a bit of surprise to existing users.  I dunno, and that
is why it was "I wonder...".

Re: [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> We do not want an ellipsis displayed following an (abbreviated) SHA-1
> value.
>
> The days when this was necessary to indicate the truncation to
> lower-level Git commands and/or the user are bygone.
>
> However, to ease the transition, the ellipsis will still be printed if
> the user (actively!) sets the environment variable PRINT_SHA1_ELLIPSIS
> to "yes" (case does not matter).

Does "(actively!)" add any value here?  For that matter, it appears
to me that "(case does not matter)" does not, either.  I thought
you'd be also reacting to Print_SHA1_Ellipsis variable, too, but the
code does not seem to be doing so (for obvious reasons).  If the users
can get what they want by setting it to "yes", that is sufficient to
say, whether your implementation accepts "Yes" as a synonym or not,
especially for something like this that we would want to remove in
an year or two.

> The transient nature of this fallback suggests that we should not prefix
> the variable by "GIT_".

Hmph.  That nature does not suggest anything like it to me.  When I
find an unfamiliar environment variable in my ~/.login I defined or
added to an existing script a few years back and forgot what it was
for, with a GIT_ prefix I would have one extra clue to help me
recall that it was once needed but no longer supported one that I
can safely remove.

I do agree that moving to an environment variable is a more useful
escape hatch for existing scripts that could be broken with this
change.

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 7d8bcc383351..e6d3a28fe26e 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -400,10 +400,17 @@ static void show_local_changes(struct object *head,
>  static void describe_detached_head(const char *msg, struct commit *commit)
>  {
>  	struct strbuf sb = STRBUF_INIT;
> +	const char *env_printsha1ellipsis = getenv("PRINT_SHA1_ELLIPSIS");
> +
>  	if (!parse_commit(commit))
>  		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
> -	fprintf(stderr, "%s %s... %s\n", msg,
> -		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
> +	if (env_printsha1ellipsis && !strcasecmp(env_printsha1ellipsis, "yes")) {
> +		fprintf(stderr, "%s %s... %s\n", msg,
> +			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
> +	} else {
> +		fprintf(stderr, "%s %s %s\n", msg,
> +			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
> +	}
>  	strbuf_release(&sb);
>  }

I would actually have expected a helper function like

int print_sha1_ellipsis(void)
{
        static int cached_result = -1; /* unknown */

	if (cached_result < 0) {
		const char *v = getenv("PRINT_SHA1_ELLIPSIS");
                cached_result = (v && !strcasecmp(v, "yes"));
	}
	return cached_result;
}

so that you can reuse that in here and in quite a different place
like in diff.c.

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> *1* We are being overly generous in t4013-diff-various.sh because we do
> not want to destroy/take apart the here-document.  Given that all this a
> temporary measure, we should get away with it.

I do not think the patch is being particularly generous.  If
anything, it is being unnecessarily sloppy by not adding new checks
to verify the updated behaviour.

The above comment mentions "destroy/take apart" the here-document,
but I do see no need to destroy anything.  All you need to do is to
enhance and extend.  For example, you could do it like so (this is
written in my e-mail client, and not an output of diff, so the
indentation etc. may be all off, but should be sufficient to
illustrate the idea):

    while read cmd
    do
            case "$cmd" in
            '' | '#'*) continue ;;
            esac
            test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
            pfx=$(printf "%04d" $test_count)
            expect="$TEST_DIRECTORY/t4013/diff.$test"
            actual="$pfx-diff.$test"
   +        case "$cmd" in
   +        X*) cmd=${cmd#X}; no_ellipses=" (no ellipses)" ;;
   +        *) no_ellipses= ;;
   +        esac
   -        test_expect_success "git $cmd" '
   +        test_expect_success "git $cmd$no_ellipses" '
                {
                        echo "\$ git $cmd"
   -                    git $cmd |
   +                    if test -n "$no_ellipses"
   +                    then
   +                            git $cmd
   +                    else
   +                            PRINT_SHA1_ELLIPSES=yes git $cmd
   +                    fi |
                        sed -e ....
        ...
    done <<\EOF
    diff-tree initial
    diff-tree -r initial
    diff-tree -r --abbrev initial
    diff-tree -r --abbrev=4 initial
   +Xdiff-tree -r --abbrev=4 initial
    ...
    EOF

There is a new and duplicated line with a prefix X for one existing
test in the above.  The idea is that the ones marked as such will
test and verify the effect of this new behaviour by not setting the
environment variable.  The expected and actual test output for the
new test will have X prefixed to it.  t4013 is arranged in such a
way that it is easy to add a new test like this---you only need to
add an expected output in a new file in t/t4013/. directory.  And
the output with these ellipses removed will be something we would
expect see in the new world (without the escape hatch environment
variable), we would need to add a new file there to record what the
expected output from the command is.

I singled out the diff-tree invocation with --abbrev=4 as an example
in the above, but in a more thorough final version, we'd need to
cover both "abbreviation with ellipses" and "abbreviation without
ellipses" output for other lines in the test case listed in the
here-document.

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Ann T Ropea <bedhanger@gmx.de> writes:
>
>> *1* We are being overly generous in t4013-diff-various.sh because we do
>> not want to destroy/take apart the here-document.  Given that all this a
>> temporary measure, we should get away with it.

So, the need to reformat the test for the future post-deprecation period is 
being deferred to the time that the PRINT_SHA1_ELLIPSIS env variable, and 
all ellipis, is removed - is that the case? Maybe it just needs saying 
plainly.

Or is the env variable being retained as a fallback 'forever'? I'm half 
guessing that it may tend toward the latter as it's an easier backward 
compatibility decision.

[apologioes this is mid thread, I'm catching up on 2 weeks of emails]

>
> I do not think the patch is being particularly generous.  If
> anything, it is being unnecessarily sloppy by not adding new checks
> to verify the updated behaviour.
>
> The above comment mentions "destroy/take apart" the here-document,
> but I do see no need to destroy anything.  All you need to do is to
> enhance and extend.  For example, you could do it like so (this is
> written in my e-mail client, and not an output of diff, so the
> indentation etc. may be all off, but should be sufficient to
> illustrate the idea):
>
>    while read cmd
>    do
>            case "$cmd" in
>            '' | '#'*) continue ;;
>            esac
>            test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
>            pfx=$(printf "%04d" $test_count)
>            expect="$TEST_DIRECTORY/t4013/diff.$test"
>            actual="$pfx-diff.$test"
>   +        case "$cmd" in
>   +        X*) cmd=${cmd#X}; no_ellipses=" (no ellipses)" ;;
>   +        *) no_ellipses= ;;
>   +        esac
>   -        test_expect_success "git $cmd" '
>   +        test_expect_success "git $cmd$no_ellipses" '
>                {
>                        echo "\$ git $cmd"
>   -                    git $cmd |
>   +                    if test -n "$no_ellipses"
>   +                    then
>   +                            git $cmd
>   +                    else
>   +                            PRINT_SHA1_ELLIPSES=yes git $cmd
>   +                    fi |
>                        sed -e ....
>        ...
>    done <<\EOF
>    diff-tree initial
>    diff-tree -r initial
>    diff-tree -r --abbrev initial
>    diff-tree -r --abbrev=4 initial
>   +Xdiff-tree -r --abbrev=4 initial
>    ...
>    EOF
>
> There is a new and duplicated line with a prefix X for one existing
> test in the above.  The idea is that the ones marked as such will
> test and verify the effect of this new behaviour by not setting the
> environment variable.  The expected and actual test output for the
> new test will have X prefixed to it.  t4013 is arranged in such a
> way that it is easy to add a new test like this---you only need to
> add an expected output in a new file in t/t4013/. directory.  And
> the output with these ellipses removed will be something we would
> expect see in the new world (without the escape hatch environment
> variable), we would need to add a new file there to record what the
> expected output from the command is.
>
> I singled out the diff-tree invocation with --abbrev=4 as an example
> in the above, but in a more thorough final version, we'd need to
> cover both "abbreviation with ellipses" and "abbreviation without
> ellipses" output for other lines in the test case listed in the
> here-document. 

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>> Ann T Ropea <bedhanger@gmx.de> writes:
>>
>>> *1* We are being overly generous in t4013-diff-various.sh because we do
>>> not want to destroy/take apart the here-document.  Given that all this a
>>> temporary measure, we should get away with it.
>
> So, the need to reformat the test for the future post-deprecation
> period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env
> variable, and all ellipis, is removed - is that the case? Maybe it
> just needs saying plainly.

And if we say it that way, it is clear that with this series, we are
shipping a new feature with a test that does not protect the output
format we claim to be the improved and preferred one.  That sounds
quite bad.

Having said that, I have already queued this to 'pu' and I do not
terribly mind to merge it down to 'next', leaving the test updates
to cover the new output format as well as the backward compatible
one at the same time for a later follow-up patch.  

I'd however hate it if I have to carry the topic in the current
shape in 'next' forever, waiting for such an update to come, that
may never materialize, and be forced to do it myself without being
explicitly asked by (and thanked for) anybody, especially because
this is not exactly my itch X-<.

> Or is the env variable being retained as a fallback 'forever'? I'm
> half guessing that it may tend toward the latter as it's an easier
> backward compatibility decision.

We do not know until this change is released to the wild, at which
time we will hear noises about the lack of expected ellipses their
(poorly written) scripts rely on and tell them to set the workaround
environment variable.  We may not hear from such people at all, in
which case we may be able to remove it within a year or so, but it
is too early to tell.

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com>
>>> Ann T Ropea <bedhanger@gmx.de> writes:
>>>
>>>> *1* We are being overly generous in t4013-diff-various.sh because we do
>>>> not want to destroy/take apart the here-document.  Given that all this 
>>>> a
>>>> temporary measure, we should get away with it.
>>
>> So, the need to reformat the test for the future post-deprecation
>> period is being deferred to the time that the PRINT_SHA1_ELLIPSIS env
>> variable, and all ellipis, is removed - is that the case? Maybe it
>> just needs saying plainly.
>
> And if we say it that way, it is clear that with this series, we are
> shipping a new feature with a test that does not protect the output
> format we claim to be the improved and preferred one.  That sounds
> quite bad.
>
> Having said that, I have already queued this to 'pu' and I do not
> terribly mind to merge it down to 'next', leaving the test updates
> to cover the new output format as well as the backward compatible
> one at the same time for a later follow-up patch.

I'd agree. I just wanted to ensure that I had the right understanding.
>
> I'd however hate it if I have to carry the topic in the current
> shape in 'next' forever, waiting for such an update to come, that
> may never materialize, and be forced to do it myself without being
> explicitly asked by (and thanked for) anybody, especially because
> this is not exactly my itch X-<.

True.
>
>> Or is the env variable being retained as a fallback 'forever'? I'm
>> half guessing that it may tend toward the latter as it's an easier
>> backward compatibility decision.
>
> We do not know until this change is released to the wild, at which
> time we will hear noises about the lack of expected ellipses their
> (poorly written) scripts rely on and tell them to set the workaround
> environment variable.  We may not hear from such people at all, in
> which case we may be able to remove it within a year or so, but it
> is too early to tell.

I was wondering if there should be a small documentation change for the env 
variable and states that it is a temporary measure for short term 
compatibility. Though I'm not sure where the 'right' place would be for it.

Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

>> We do not know until this change is released to the wild, at which
>> time we will hear noises about the lack of expected ellipses their
>> (poorly written) scripts rely on and tell them to set the workaround
>> environment variable.  We may not hear from such people at all, in
>> which case we may be able to remove it within a year or so, but it
>> is too early to tell.
>
> I was wondering if there should be a small documentation change for
> the env variable and states that it is a temporary measure for short
> term compatibility. Though I'm not sure where the 'right' place would
> be for it.

We list environment variables that applies git-wide in git(1), I
think.  If this is going to be only applicable for the "diff"
family, Documentation/diff-format.txt may be a better place, and
in that case the name of the environment variable may need to
include a word DIFF somewhere.

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Ann T Ropea]

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>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 diff.c | 23 ++++++++++++++++++++++-
 diff.h |  6 ++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 0763e89263ef..8395623acdb1 100644
--- a/diff.c
+++ b/diff.c
@@ -4902,14 +4902,20 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
 	int abblen;
 	const char *abbrev;
 
+	/* 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 (!print_sha1_ellipsis())
+		return abbrev;
+
 	abblen = strlen(abbrev);
 
 	/*
-	 * In well-behaved cases, where the abbbreviated result is the
+	 * In well-behaved cases, where the abbreviated result is the
 	 * same as the requested length, append three dots after the
 	 * abbreviation (hence the whole logic is limited to the case
 	 * where abblen < 37); when the actual abbreviated result is a
@@ -6067,3 +6073,18 @@ void setup_diff_pager(struct diff_options *opt)
 	    check_pager_config("diff") != 0)
 		setup_pager();
 }
+
+int print_sha1_ellipsis(void)
+{
+	/*
+	 * Determine if the calling environment contains the variable
+	 * GIT_PRINT_SHA1_ELLIPSIS set to "yes".
+	 */
+	static int cached_result = -1; /* unknown */
+
+	if (cached_result < 0) {
+		const char *v = getenv("GIT_PRINT_SHA1_ELLIPSIS");
+		cached_result = (v && !strcasecmp(v, "yes"));
+	}
+	return cached_result;
+}
diff --git a/diff.h b/diff.h
index 0fb18dd735b5..a98c9e1cc367 100644
--- a/diff.h
+++ b/diff.h
@@ -438,4 +438,10 @@ extern void print_stat_summary(FILE *fp, int files,
 			       int insertions, int deletions);
 extern void setup_diff_pager(struct diff_options *);
 
+/*
+ * Should we print an ellipsis after an abbreviated SHA-1 value
+ * when doing diff-raw output or indicating a detached HEAD?
+ */
+extern int print_sha1_ellipsis(void);
+
 #endif /* DIFF_H */
-- 
2.13.6

Re: [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes".

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 builtin/checkout.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7d8bcc383351..d260fb925cac 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,10 +400,16 @@ static void show_local_changes(struct object *head,
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (print_sha1_ellipsis()) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
-- 
2.13.6

Re: [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis

[Ann T Ropea]

Confusing the ellipsis with the three-dot operator should be made as
difficult as possible.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 Documentation/user-manual.txt | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 3a03e63eb0d8..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
@@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this
 
 If you run `git branch` at this point, you'll see that Git has
 temporarily moved you in "(no branch)". HEAD is now detached from any
-branch and points directly to a commit (with commit id 65934...) that
+branch and points directly to a commit (with commit id 65934) that
 is reachable from "master" but not from v2.6.18. Compile and test it,
 and see whether it crashes. Assume it does crash. Then:
 
@@ -549,14 +549,14 @@ says "bisect".  Choose a safe-looking commit nearby, note its commit
 id, and check it out with:
 
 -------------------------------------------------
-$ git reset --hard fb47ddb2db...
+$ git reset --hard fb47ddb2db
 -------------------------------------------------
 
 then test, run `bisect good` or `bisect bad` as appropriate, and
 continue.
 
 Instead of `git bisect visualize` and then `git reset --hard
-fb47ddb2db...`, you might just want to tell Git that you want to skip
+fb47ddb2db`, you might just want to tell Git that you want to skip
 the current commit:
 
 -------------------------------------------------
@@ -3416,7 +3416,7 @@ commit abc
 Author:
 Date:
 ...
-:100644 100644 4b9458b... newsha... M somedirectory/myfile
+:100644 100644 4b9458b newsha M somedirectory/myfile
 
 
 commit xyz
@@ -3424,7 +3424,7 @@ Author:
 Date:
 
 ...
-:100644 100644 oldsha... 4b9458b... M somedirectory/myfile
+:100644 100644 oldsha 4b9458b M somedirectory/myfile
 ------------------------------------------------
 
 This tells you that the immediately following version of the file was
@@ -3449,7 +3449,7 @@ and your repository is good again!
 $ git log --raw --all
 ------------------------------------------------
 
-and just looked for the sha of the missing object (4b9458b..) in that
+and just looked for the sha of the missing object (4b9458b) in that
 whole thing. It's up to you--Git does *have* a lot of information, it is
 just missing one particular blob version.
 
@@ -4114,9 +4114,9 @@ program, e.g.  `diff3`, `merge`, or Git's own merge-file, on
 the blob objects from these three stages yourself, like this:
 
 ------------------------------------------------
-$ git cat-file blob 263414f... >hello.c~1
-$ git cat-file blob 06fa6a2... >hello.c~2
-$ git cat-file blob cc44c73... >hello.c~3
+$ git cat-file blob 263414f >hello.c~1
+$ git cat-file blob 06fa6a2 >hello.c~2
+$ git cat-file blob cc44c73 >hello.c~3
 $ git merge-file hello.c~2 hello.c~1 hello.c~3
 ------------------------------------------------
 
@@ -4374,7 +4374,7 @@ $ git log --no-merges t/
 ------------------------
 
 In the pager (`less`), just search for "bundle", go a few lines back,
-and see that it is in commit 18449ab0...  Now just copy this object name,
+and see that it is in commit 18449ab0.  Now just copy this object name,
 and paste it into the command line
 
 -------------------
-- 
2.13.6

Re: [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot").

[Ann T Ropea]

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..dfcc49c72c0f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
-- 
2.13.6

Re: [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS

[Ann T Ropea]

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 Documentation/git.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 483a1f35475e..395c88c8a31f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -731,6 +731,15 @@ corresponding standard handle, and if `GIT_REDIRECT_STDERR` is
 `2>&1`, standard error will be redirected to the same handle as
 standard output.
 
+`GIT_PRINT_SHA1_ELLIPSIS` (deprecated)::
+	If set to `yes`, print an ellipsis following an
+	(abbreviated) SHA-1 value.  This affects indications of
+	detached HEADs (linkgit:git-checkout[1]) and the raw
+	diff output (linkgit:git-diff[1]).  Printing an
+	ellipsis in the cases mentioned is no longer considered
+	adequate and support for it is likely to be removed in the
+	foreseeable future (along with the variable).
+
 Discussion[[Discussion]]
 ------------------------
 
-- 
2.13.6

Re: [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values

[Ann T Ropea]

At the moment, this patch's concern is to not break existing tests.

A follow-up patch (series) will rectify the situation by covering the
new output format as well as the backward compatible one.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
 t/t3040-subprojects-basic.sh | 2 +-
 t/t4013-diff-various.sh      | 2 +-
 t/t9300-fast-import.sh       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
index 0a4ff6d824a0..b81eb5fd6ffa 100755
--- a/t/t3040-subprojects-basic.sh
+++ b/t/t3040-subprojects-basic.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' '
 	git update-index --add sub1 &&
 	git add sub2 &&
 	git commit -q -m "subprojects added" &&
-	git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
+	GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
 	git branch save HEAD &&
 	cat >expected <<-\EOF &&
 	:000000 160000 00000... A	sub1
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index c515e3e53fee..9bed64d53e01 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -131,7 +131,7 @@ do
 	test_expect_success "git $cmd" '
 		{
 			echo "\$ git $cmd"
-			git $cmd |
+			GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd |
 			sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \
 			    -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/"
 			echo "\$"
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d47560b6343d..e4d06accc458 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' '
 	EXPECT_END
 
 	git fast-import <input &&
-	git diff-tree --abbrev --raw L^ L >output &&
+	GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output &&
 	test_cmp expect output
 '
 
-- 
2.13.6

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> 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>
> ---
> v2: rename patch series & focus on removal of ellipses
> v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
> v4: improve env var handling (rename, helper func to query, docu)

Thanks for sticking with this topic---very much appreciated, as we
saw many newcomers get tired of doing repeated polishing and abandon
their topic in the past.  I have to go offline now, but will comment
later when I come back.

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Thanks for sticking with this topic---very much appreciated, as we
> saw many newcomers get tired of doing repeated polishing and abandon
> their topic in the past.  I have to go offline now, but will comment
> later when I come back.

Regarding the overall structure of the series, I think 1/6 will
break tests and the breakage will stay until it is fixed at the
last step.  Also, now print_sha1_ellipsis() is prominently not about
"diff", it may want to move out of "diff.[ch]".

So, perhaps this may be a better structure:

 - 1/6: What we see in [v4 4/6] as an independent typofix.

 - 2/6: What we see in [v4 3/6] that is not about the output from
        "git checkout" (e.g. the use of ellipsis to shorten the
        example input the user gives, as if the user is using full
        40-hex and we are not bothering to show everything in the
        documentation), with "there is no need to use full 40-hex to
        identify the object names like the examples hint at by
        omitting the tail part of an object name as if that has to
        be spelled out but the example omits them only for
        brevity. Give examples using an abbreviated object names
        without ellipses just like how people do in real life", or
        something like that, as an explanation.

 - 3/6: Introduce a helper print_sha1_ellipsis() that pays attention
        to GIT_PRINT_SHA1_ELLIPSIS environment, and prepares the
        tests to unconditionally set it for the test pieces that
        will be broken once the code stops showing the extra dots by
        default.  What we see in [v4 5/6] may also want to be rolled
        into this patch.

        Nobody calls print_sha1_ellipsis() function yet at this
        step.  cache.h and environment.c may be a better place for
        the declaration and definition of the helper.

	Mention that the removal of these dots is merely a plan at
        this step and has not happened yet but soon will be in the
        proposed log message.

 - 4/6: What we see in [v4 2/6], optionally with two additional
        tests that looks at the output with and without the
        environment variable (it is optional because the output is
        purely for human consumption sent to the standard error
        stream).  Parts of [v4 3/6] about the output from the "git
        checkout" command also belongs to this step.

 - 5/6: What we see in [v4 1/6] (except for introduction of the
        helper, which already happened in an earlier step).  Move
        the promise of covering this new output format with a follow
        up series to the proposed log message of this step from [v4
        6/6].

 - 6/6: Realize the promise made in 5/6.

	Alternatively, just like the step 3/6 above prepares the
        tests for "upcoming" feature without exercising that
        preparation, a part of this can be split to allow annotating
        the "here-document" test vectors with "does this test expect
        that ellipsis will be missing in the output?" (i.e. change
        the logic in the loop that parses "<<EOF") without adding
        anything to the test vectors, and made into a separate step
        between 4/6 and 5/6 above.  Then we can include an update to
        the test in 5/6 that looks at the new output format without
        the environment in 5/6, which makes it clearer what the
        change is doing when viewing 5/6 alone.


For rererence, here is what I just whipped up that would come in
between the steps 4/6 and 5/6 in the above outline (i.e. split from
6/6 to be placed before 5/6 happens).

-- >8 --
Subject: [PATCH] t4013: prepare for upcoming "diff --raw --abbrev" format change

Most of the t4013 tests go through a list of sample command lines,
and each of them is executed and its output compared with an
expected one stored in t4013/ directory.  Allow these lines to begin
with a colon followed by magic word(s) so that test conditions can
easily be tweaked.

The expected use that will happen in later steps of this is to run
tests expecting the traditional output and run the same test without
the GIT_PRINT_SHA1_ELLIPSIS=yes environment exported for (perhaps
some of) them, which will have to expect different output.  Since
all of the existing tests are meant to run with the environment,
use the magic word "noellipses" to cause the variable not to be set
and exported.

As this step does not add any new test with the magic word, all
tests still run with the environment variable, expecting the
traditional output, but it will change soon.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4013-diff-various.sh | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9bed64d53e..7288b54045 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -118,20 +118,37 @@ test_expect_success setup '
 EOF
 
 V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g')
-while read cmd
+while read magic cmd
 do
-	case "$cmd" in
-	'' | '#'*) continue ;;
+	case "$magic" in
+	'' | '#'*)
+		continue ;;
+	:*)
+		magic=${magic#:}
+		label="$magic-$cmd"
+		case "$magic" in
+		noellipses) ;;
+		*)
+			die "bug in t4103: unknown magic $magic" ;;
+		esac ;;
+	*)
+		cmd="$magic $cmd" magic=
+		label="$cmd" ;;
 	esac
-	test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
+	test=$(echo "$label" | sed -e 's|[/ ][/ ]*|_|g')
 	pfx=$(printf "%04d" $test_count)
 	expect="$TEST_DIRECTORY/t4013/diff.$test"
 	actual="$pfx-diff.$test"
 
 	test_expect_success "git $cmd" '
 		{
-			echo "\$ git $cmd"
-			GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd |
+			echo "$ git $cmd"
+			case "$magic" in
+			"")
+				GIT_PRINT_SHA1_ELLIPSIS=yes git $cmd ;;
+			noellipses)
+				git $cmd ;;
+			esac |
 			sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \
 			    -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/"
 			echo "\$"
-- 
2.15.0-479-ge11ee266c9

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

... and this is a sample of the "remainder" of 6/6 outlined in the
previous message, to become part of 5/6 to show how the new output
would look like in a test form.

 t/t4013-diff-various.sh                                        | 1 +
 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial | 6 ++++++
 2 files changed, 7 insertions(+)
 create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 7288b54045..8b5474a087 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -178,6 +178,7 @@ diff-tree --root --abbrev initial
 diff-tree --root -r initial
 diff-tree --root -r --abbrev initial
 diff-tree --root -r --abbrev=4 initial
+:noellipses diff-tree --root -r --abbrev=4 initial
 diff-tree -p initial
 diff-tree --root -p initial
 diff-tree --patch-with-stat initial
diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
new file mode 100644
index 0000000000..26fbfeb177
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
@@ -0,0 +1,6 @@
+$ git diff-tree --root -r --abbrev=4 initial
+444ac553ac7612cc88969031b02b3767fb8a353a
+:000000 100644 0000 35d2 A	dir/sub
+:000000 100644 0000 01e7 A	file0
+:000000 100644 0000 01e7 A	file2
+$
-- 
2.15.0-479-ge11ee266c9

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>  - 5/6: What we see in [v4 1/6] (except for introduction of the
>         helper, which already happened in an earlier step).  Move
>         the promise of covering this new output format with a follow
>         up series to the proposed log message of this step from [v4
>         6/6].
>
>  - 6/6: Realize the promise made in 5/6.
> ...
> For rererence, here is what I just whipped up that would come in
> between the steps 4/6 and 5/6 in the above outline (i.e. split from
> 6/6 to be placed before 5/6 happens).

Just to be sure, I wrote this on top of [v4 1-6/6], so you'd see
that its preimage already knows about GIT_PRINT_SHA1_ELLIPSIS=yes.
In a real reroll of the series that follow the above reorganized
structure, of course that would not appear as "-" line in the patch
;-)

Thanks.

Re: [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot").

[Ann T Ropea]

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 Documentation/revisions.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 61277469c874..dfcc49c72c0f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -271,7 +271,7 @@ The '..' (two-dot) Range Notation::
  for commits that are reachable from r2 excluding those that are reachable
  from r1 by '{caret}r1 r2' and it can be written as 'r1..r2'.
 
-The '...' (three dot) Symmetric Difference Notation::
+The '...' (three-dot) Symmetric Difference Notation::
  A similar notation 'r1\...r2' is called symmetric difference
  of 'r1' and 'r2' and is defined as
  'r1 r2 --not $(git merge-base --all r1 r2)'.
-- 
2.13.6

Re: [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis

[Ann T Ropea]

There is no need to use full 40-hex to identify the object names like
the examples hint at by omitting the tail part of an object name as if
that has to be spelled out but the example omits them only for brevity.
Give examples using abbreviated object names without ellipses just like
how people do in real life.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 Documentation/user-manual.txt | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 3a03e63eb0d8..497e82e88dd0 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -508,7 +508,7 @@ Bisecting: 3537 revisions left to test after this
 
 If you run `git branch` at this point, you'll see that Git has
 temporarily moved you in "(no branch)". HEAD is now detached from any
-branch and points directly to a commit (with commit id 65934...) that
+branch and points directly to a commit (with commit id 65934) that
 is reachable from "master" but not from v2.6.18. Compile and test it,
 and see whether it crashes. Assume it does crash. Then:
 
@@ -549,14 +549,14 @@ says "bisect".  Choose a safe-looking commit nearby, note its commit
 id, and check it out with:
 
 -------------------------------------------------
-$ git reset --hard fb47ddb2db...
+$ git reset --hard fb47ddb2db
 -------------------------------------------------
 
 then test, run `bisect good` or `bisect bad` as appropriate, and
 continue.
 
 Instead of `git bisect visualize` and then `git reset --hard
-fb47ddb2db...`, you might just want to tell Git that you want to skip
+fb47ddb2db`, you might just want to tell Git that you want to skip
 the current commit:
 
 -------------------------------------------------
@@ -3416,7 +3416,7 @@ commit abc
 Author:
 Date:
 ...
-:100644 100644 4b9458b... newsha... M somedirectory/myfile
+:100644 100644 4b9458b newsha M somedirectory/myfile
 
 
 commit xyz
@@ -3424,7 +3424,7 @@ Author:
 Date:
 
 ...
-:100644 100644 oldsha... 4b9458b... M somedirectory/myfile
+:100644 100644 oldsha 4b9458b M somedirectory/myfile
 ------------------------------------------------
 
 This tells you that the immediately following version of the file was
@@ -3449,7 +3449,7 @@ and your repository is good again!
 $ git log --raw --all
 ------------------------------------------------
 
-and just looked for the sha of the missing object (4b9458b..) in that
+and just looked for the sha of the missing object (4b9458b) in that
 whole thing. It's up to you--Git does *have* a lot of information, it is
 just missing one particular blob version.
 
@@ -4114,9 +4114,9 @@ program, e.g.  `diff3`, `merge`, or Git's own merge-file, on
 the blob objects from these three stages yourself, like this:
 
 ------------------------------------------------
-$ git cat-file blob 263414f... >hello.c~1
-$ git cat-file blob 06fa6a2... >hello.c~2
-$ git cat-file blob cc44c73... >hello.c~3
+$ git cat-file blob 263414f >hello.c~1
+$ git cat-file blob 06fa6a2 >hello.c~2
+$ git cat-file blob cc44c73 >hello.c~3
 $ git merge-file hello.c~2 hello.c~1 hello.c~3
 ------------------------------------------------
 
@@ -4374,7 +4374,7 @@ $ git log --no-merges t/
 ------------------------
 
 In the pager (`less`), just search for "bundle", go a few lines back,
-and see that it is in commit 18449ab0...  Now just copy this object name,
+and see that it is in commit 18449ab0.  Now just copy this object name,
 and paste it into the command line
 
 -------------------
-- 
2.13.6

Re: [PATCH v5 3/7] print_sha1_ellipsis: introduce helper

[Ann T Ropea]

Introduce a helper print_sha1_ellipsis() that pays attention to the
GIT_PRINT_SHA1_ELLIPSIS environment variable, and prepare the tests to
unconditionally set it for the test pieces that will be broken once the code
stops showing the extra dots by default.

The removal of these dots is merely a plan at this step and has not happened
yet but soon will.

Document GIT_PRINT_SHA1_ELLIPSIS.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 Documentation/git.txt        |  9 +++++++++
 cache.h                      |  6 ++++++
 environment.c                | 15 +++++++++++++++
 t/t3040-subprojects-basic.sh |  2 +-
 t/t4013-diff-various.sh      |  2 +-
 t/t9300-fast-import.sh       |  2 +-
 6 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 483a1f35475e..395c88c8a31f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -731,6 +731,15 @@ corresponding standard handle, and if `GIT_REDIRECT_STDERR` is
 `2>&1`, standard error will be redirected to the same handle as
 standard output.
 
+`GIT_PRINT_SHA1_ELLIPSIS` (deprecated)::
+	If set to `yes`, print an ellipsis following an
+	(abbreviated) SHA-1 value.  This affects indications of
+	detached HEADs (linkgit:git-checkout[1]) and the raw
+	diff output (linkgit:git-diff[1]).  Printing an
+	ellipsis in the cases mentioned is no longer considered
+	adequate and support for it is likely to be removed in the
+	foreseeable future (along with the variable).
+
 Discussion[[Discussion]]
 ------------------------
 
diff --git a/cache.h b/cache.h
index 2e143450514c..dc30f6b92025 100644
--- a/cache.h
+++ b/cache.h
@@ -1958,4 +1958,10 @@ void sleep_millisec(int millisec);
  */
 void safe_create_dir(const char *dir, int share);
 
+/*
+ * Should we print an ellipsis after an abbreviated SHA-1 value
+ * when doing diff-raw output or indicating a detached HEAD?
+ */
+extern int print_sha1_ellipsis(void);
+
 #endif /* CACHE_H */
diff --git a/environment.c b/environment.c
index 8fa032f30742..63ac38a46f8f 100644
--- a/environment.c
+++ b/environment.c
@@ -344,3 +344,18 @@ int use_optional_locks(void)
 {
 	return git_env_bool(GIT_OPTIONAL_LOCKS_ENVIRONMENT, 1);
 }
+
+int print_sha1_ellipsis(void)
+{
+	/*
+	 * Determine if the calling environment contains the variable
+	 * GIT_PRINT_SHA1_ELLIPSIS set to "yes".
+	 */
+	static int cached_result = -1; /* unknown */
+
+	if (cached_result < 0) {
+		const char *v = getenv("GIT_PRINT_SHA1_ELLIPSIS");
+		cached_result = (v && !strcasecmp(v, "yes"));
+	}
+	return cached_result;
+}
diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
index 0a4ff6d824a0..b81eb5fd6ffa 100755
--- a/t/t3040-subprojects-basic.sh
+++ b/t/t3040-subprojects-basic.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup: create subprojects' '
 	git update-index --add sub1 &&
 	git add sub2 &&
 	git commit -q -m "subprojects added" &&
-	git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
+	GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
 	git branch save HEAD &&
 	cat >expected <<-\EOF &&
 	:000000 160000 00000... A	sub1
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index c515e3e53fee..9bed64d53e01 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -131,7 +131,7 @@ do
 	test_expect_success "git $cmd" '
 		{
 			echo "\$ git $cmd"
-			git $cmd |
+			GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd |
 			sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \
 			    -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/"
 			echo "\$"
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d47560b6343d..e4d06accc458 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -876,7 +876,7 @@ test_expect_success 'L: verify internal tree sorting' '
 	EXPECT_END
 
 	git fast-import <input &&
-	git diff-tree --abbrev --raw L^ L >output &&
+	GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev --raw L^ L >output &&
 	test_cmp expect output
 '
 
-- 
2.13.6

Re: [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes".

Correct documentation with respect to what describe_detached_head prints
when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above.

Add tests for the old and new behaviour.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 Documentation/user-manual.txt |   2 +-
 builtin/checkout.c            |  10 +++-
 t/t2020-checkout-detach.sh    | 106 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 497e82e88dd0..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3faae382de4f..b0499542158f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,10 +400,16 @@ static void show_local_changes(struct object *head,
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (print_sha1_ellipsis()) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index fbb4ee9bb42d..9d42e38ed905 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -186,4 +186,110 @@ test_expect_success 'no advice given for explicit detached head state' '
 	test_cmp expect.no-advice actual
 '
 
+# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS
+
+# The first detach operation is more chatty than the following ones.
+cat > 1st_detach <<'EOF'
+Note: checking out 'HEAD^'.
+
+You are in 'detached HEAD' state. You can look around, make experimental
+changes and commit them, and you can discard any commits you make in this
+state without impacting any branches by performing another checkout.
+
+If you want to create a new branch to retain commits you create, you may
+do so (now or later) by using -b with the checkout command again. Example:
+
+  git checkout -b <new-branch-name>
+
+HEAD is now at 7c7cd714e262 three
+EOF
+# The remaining ones just show info about previous and current HEADs.
+cat > 2nd_detach <<'EOF'
+Previous HEAD position was 7c7cd714e262 three
+HEAD is now at 139b20d8e6c5 two
+EOF
+cat > 3rd_detach <<'EOF'
+Previous HEAD position was 139b20d8e6c5 two
+HEAD is now at d79ce1670bdc one
+EOF
+test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' '
+	reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Various ways of *not* asking for ellipses
+
+	unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# We only have four commits, but we can re-use them
+	reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Make no mention of the env var at all
+	git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=nein && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	true
+'
+
+# The first detach operation is more chatty than the following ones.
+cat > 1st_detach <<'EOF'
+Note: checking out 'HEAD^'.
+
+You are in 'detached HEAD' state. You can look around, make experimental
+changes and commit them, and you can discard any commits you make in this
+state without impacting any branches by performing another checkout.
+
+If you want to create a new branch to retain commits you create, you may
+do so (now or later) by using -b with the checkout command again. Example:
+
+  git checkout -b <new-branch-name>
+
+HEAD is now at 7c7cd714e262... three
+EOF
+# The remaining ones just show info about previous and current HEADs.
+cat > 2nd_detach <<'EOF'
+Previous HEAD position was 7c7cd714e262... three
+HEAD is now at 139b20d8e6c5... two
+EOF
+cat > 3rd_detach <<'EOF'
+Previous HEAD position was 139b20d8e6c5... two
+HEAD is now at d79ce1670bdc... one
+EOF
+test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' '
+	reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Various ways of asking for ellipses...
+
+	GIT_PRINT_SHA1_ELLIPSIS="yes" && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 2nd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&
+	check_detached &&
+	test_cmp 3rd_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	true
+'
+
 test_done
-- 
2.13.6

Re: [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change

[Ann T Ropea]

Most of the t4013 tests go through a list of sample command lines,
and each of them is executed and its output compared with an
expected one stored in t4013/ directory.  Allow these lines to begin
with a colon followed by magic word(s) so that test conditions can
easily be tweaked.

The expected use that will happen in later steps of this is to run
tests expecting the traditional output and run the same test without
the GIT_PRINT_SHA1_ELLIPSIS=yes environment exported for (perhaps
some of) them, which will have to expect different output.  Since
all of the existing tests are meant to run with the environment,
use the magic word "noellipses" to cause the variable not to be set
and exported.

As this step does not add any new test with the magic word, all
tests still run with the environment variable, expecting the
traditional output, but it will change soon.

Based-on-patch-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 t/t4013-diff-various.sh | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9bed64d53e01..7288b540450f 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -118,20 +118,37 @@ test_expect_success setup '
 EOF
 
 V=$(git version | sed -e 's/^git version //' -e 's/\./\\./g')
-while read cmd
+while read magic cmd
 do
-	case "$cmd" in
-	'' | '#'*) continue ;;
+	case "$magic" in
+	'' | '#'*)
+		continue ;;
+	:*)
+		magic=${magic#:}
+		label="$magic-$cmd"
+		case "$magic" in
+		noellipses) ;;
+		*)
+			die "bug in t4103: unknown magic $magic" ;;
+		esac ;;
+	*)
+		cmd="$magic $cmd" magic=
+		label="$cmd" ;;
 	esac
-	test=$(echo "$cmd" | sed -e 's|[/ ][/ ]*|_|g')
+	test=$(echo "$label" | sed -e 's|[/ ][/ ]*|_|g')
 	pfx=$(printf "%04d" $test_count)
 	expect="$TEST_DIRECTORY/t4013/diff.$test"
 	actual="$pfx-diff.$test"
 
 	test_expect_success "git $cmd" '
 		{
-			echo "\$ git $cmd"
-			GIT_PRINT_SHA1_ELLIPSIS="yes" git $cmd |
+			echo "$ git $cmd"
+			case "$magic" in
+			"")
+				GIT_PRINT_SHA1_ELLIPSIS=yes git $cmd ;;
+			noellipses)
+				git $cmd ;;
+			esac |
 			sed -e "s/^\\(-*\\)$V\\(-*\\)\$/\\1g-i-t--v-e-r-s-i-o-n\2/" \
 			    -e "s/^\\(.*mixed; boundary=\"-*\\)$V\\(-*\\)\"\$/\\1g-i-t--v-e-r-s-i-o-n\2\"/"
 			echo "\$"
-- 
2.13.6

Re: [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Ann T Ropea]

Neither Git nor the user are in need of this (visual) aid anymore, but
we must offer a transition period.

A follow-up patch (series) will rectify the situation by covering the
new output format as well as the backward compatible one.

Also, fix a typo: "abbbreviated" ---> "abbreviated".

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 diff.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 2ebe2227b467..a187c670df6b 100644
--- a/diff.c
+++ b/diff.c
@@ -4902,14 +4902,20 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
 	int abblen;
 	const char *abbrev;
 
+	/* 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 (!print_sha1_ellipsis())
+		return abbrev;
+
 	abblen = strlen(abbrev);
 
 	/*
-	 * In well-behaved cases, where the abbbreviated result is the
+	 * In well-behaved cases, where the abbreviated result is the
 	 * same as the requested length, append three dots after the
 	 * abbreviation (hence the whole logic is limited to the case
 	 * where abblen < 37); when the actual abbreviated result is a
-- 
2.13.6

Re: [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw

[Ann T Ropea]

Use newly-introduced finely-grained control to teach the diff-family to
honor the new environment GIT_PRINT_SHA1_ELLIPSIS and remove the
ellipses when it is not set.

Mentored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
 t/t4013-diff-various.sh                            | 16 ++++++++-
 ...ff.noellipses-diff-tree_--root_--abbrev_initial |  6 ++++
 ...ellipses-diff-tree_--root_-r_--abbrev=4_initial |  6 ++++
 ...noellipses-diff-tree_--root_-r_--abbrev_initial |  6 ++++
 .../diff.noellipses-diff-tree_-c_--abbrev_master   |  5 +++
 ...ipses-diff_--no-index_--raw_--abbrev=4_dir2_dir |  3 ++
 .../diff.noellipses-diff_--no-index_--raw_dir2_dir |  3 ++
 ...ellipses-diff_--patch-with-raw_-r_initial..side | 36 +++++++++++++++++++
 ....noellipses-diff_--patch-with-raw_initial..side | 36 +++++++++++++++++++
 .../diff.noellipses-diff_--raw_--abbrev=4_initial  |  6 ++++
 t/t4013/diff.noellipses-diff_--raw_initial         |  6 ++++
 t/t4013/diff.noellipses-show_--patch-with-raw_side | 42 ++++++++++++++++++++++
 t/t4013/diff.noellipses-whatchanged_--root_master  | 42 ++++++++++++++++++++++
 t/t4013/diff.noellipses-whatchanged_-SF_master     |  9 +++++
 t/t4013/diff.noellipses-whatchanged_master         | 32 +++++++++++++++++
 15 files changed, 253 insertions(+), 1 deletion(-)
 create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial
 create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
 create mode 100644 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial
 create mode 100644 t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master
 create mode 100644 t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir
 create mode 100644 t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir
 create mode 100644 t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side
 create mode 100644 t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side
 create mode 100644 t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial
 create mode 100644 t/t4013/diff.noellipses-diff_--raw_initial
 create mode 100644 t/t4013/diff.noellipses-show_--patch-with-raw_side
 create mode 100644 t/t4013/diff.noellipses-whatchanged_--root_master
 create mode 100644 t/t4013/diff.noellipses-whatchanged_-SF_master
 create mode 100644 t/t4013/diff.noellipses-whatchanged_master

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 7288b540450f..f10798b2dff3 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -140,7 +140,7 @@ do
 	expect="$TEST_DIRECTORY/t4013/diff.$test"
 	actual="$pfx-diff.$test"
 
-	test_expect_success "git $cmd" '
+	test_expect_success "git $cmd # magic is ${magic:-"(not used)"}" '
 		{
 			echo "$ git $cmd"
 			case "$magic" in
@@ -175,9 +175,12 @@ diff-tree -r --abbrev initial
 diff-tree -r --abbrev=4 initial
 diff-tree --root initial
 diff-tree --root --abbrev initial
+:noellipses diff-tree --root --abbrev initial
 diff-tree --root -r initial
 diff-tree --root -r --abbrev initial
+:noellipses diff-tree --root -r --abbrev initial
 diff-tree --root -r --abbrev=4 initial
+:noellipses diff-tree --root -r --abbrev=4 initial
 diff-tree -p initial
 diff-tree --root -p initial
 diff-tree --patch-with-stat initial
@@ -226,6 +229,7 @@ diff-tree -p master
 diff-tree -p -m master
 diff-tree -c master
 diff-tree -c --abbrev master
+:noellipses diff-tree -c --abbrev master
 diff-tree --cc master
 # stat only should show the diffstat with the first parent
 diff-tree -c --stat master
@@ -272,8 +276,10 @@ rev-list --parents HEAD
 rev-list --children HEAD
 
 whatchanged master
+:noellipses whatchanged master
 whatchanged -p master
 whatchanged --root master
+:noellipses whatchanged --root master
 whatchanged --root -p master
 whatchanged --patch-with-stat master
 whatchanged --root --patch-with-stat master
@@ -283,6 +289,7 @@ whatchanged --root -c --patch-with-stat --summary master
 # improved by Timo's patch
 whatchanged --root --cc --patch-with-stat --summary master
 whatchanged -SF master
+:noellipses whatchanged -SF master
 whatchanged -SF -p master
 
 log --patch-with-stat master -- dir/
@@ -301,6 +308,7 @@ show --stat side
 show --stat --summary side
 show --patch-with-stat side
 show --patch-with-raw side
+:noellipses show --patch-with-raw side
 show --patch-with-stat --summary side
 
 format-patch --stdout initial..side
@@ -328,8 +336,10 @@ diff -r --stat initial..side
 diff initial..side
 diff --patch-with-stat initial..side
 diff --patch-with-raw initial..side
+:noellipses diff --patch-with-raw initial..side
 diff --patch-with-stat -r initial..side
 diff --patch-with-raw -r initial..side
+:noellipses diff --patch-with-raw -r initial..side
 diff --name-status dir2 dir
 diff --no-index --name-status dir2 dir
 diff --no-index --name-status -- dir2 dir
@@ -342,10 +352,14 @@ diff --dirstat initial rearrange
 diff --dirstat-by-file initial rearrange
 # No-index --abbrev and --no-abbrev
 diff --raw initial
+:noellipses diff --raw initial
 diff --raw --abbrev=4 initial
+:noellipses diff --raw --abbrev=4 initial
 diff --raw --no-abbrev initial
 diff --no-index --raw dir2 dir
+:noellipses diff --no-index --raw dir2 dir
 diff --no-index --raw --abbrev=4 dir2 dir
+:noellipses diff --no-index --raw --abbrev=4 dir2 dir
 diff --no-index --raw --no-abbrev dir2 dir
 EOF
 
diff --git a/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial b/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial
new file mode 100644
index 000000000000..4bdad4072ebf
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_--root_--abbrev_initial
@@ -0,0 +1,6 @@
+$ git diff-tree --root --abbrev initial
+444ac553ac7612cc88969031b02b3767fb8a353a
+:000000 040000 0000000 da7a33f A	dir
+:000000 100644 0000000 01e79c3 A	file0
+:000000 100644 0000000 01e79c3 A	file2
+$
diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
new file mode 100644
index 000000000000..26fbfeb177be
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
@@ -0,0 +1,6 @@
+$ git diff-tree --root -r --abbrev=4 initial
+444ac553ac7612cc88969031b02b3767fb8a353a
+:000000 100644 0000 35d2 A	dir/sub
+:000000 100644 0000 01e7 A	file0
+:000000 100644 0000 01e7 A	file2
+$
diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial
new file mode 100644
index 000000000000..2ac856119190
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev_initial
@@ -0,0 +1,6 @@
+$ git diff-tree --root -r --abbrev initial
+444ac553ac7612cc88969031b02b3767fb8a353a
+:000000 100644 0000000 35d242b A	dir/sub
+:000000 100644 0000000 01e79c3 A	file0
+:000000 100644 0000000 01e79c3 A	file2
+$
diff --git a/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master b/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master
new file mode 100644
index 000000000000..bb80f013b37d
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_-c_--abbrev_master
@@ -0,0 +1,5 @@
+$ git diff-tree -c --abbrev master
+59d314ad6f356dd08601a4cd5e530381da3e3c64
+::100644 100644 100644 cead32e 7289e35 992913c MM	dir/sub
+::100644 100644 100644 b414108 f4615da 10a8a9f MM	file0
+$
diff --git a/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir b/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir
new file mode 100644
index 000000000000..41b7baf0a534
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--no-index_--raw_--abbrev=4_dir2_dir
@@ -0,0 +1,3 @@
+$ git diff --no-index --raw --abbrev=4 dir2 dir
+:000000 100644 0000 0000 A	dir/sub
+$
diff --git a/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir b/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir
new file mode 100644
index 000000000000..0cf3a3efea83
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--no-index_--raw_dir2_dir
@@ -0,0 +1,3 @@
+$ git diff --no-index --raw dir2 dir
+:000000 100644 0000000 0000000 A	dir/sub
+$
diff --git a/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side b/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side
new file mode 100644
index 000000000000..8d1f1e372199
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--patch-with-raw_-r_initial..side
@@ -0,0 +1,36 @@
+$ git diff --patch-with-raw -r initial..side
+:100644 100644 35d242b 7289e35 M	dir/sub
+:100644 100644 01e79c3 f4615da M	file0
+:000000 100644 0000000 7289e35 A	file3
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+$
diff --git a/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side b/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side
new file mode 100644
index 000000000000..50d8aee4f7f3
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--patch-with-raw_initial..side
@@ -0,0 +1,36 @@
+$ git diff --patch-with-raw initial..side
+:100644 100644 35d242b 7289e35 M	dir/sub
+:100644 100644 01e79c3 f4615da M	file0
+:000000 100644 0000000 7289e35 A	file3
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+$
diff --git a/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial b/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial
new file mode 100644
index 000000000000..8ae44d6c8365
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--raw_--abbrev=4_initial
@@ -0,0 +1,6 @@
+$ git diff --raw --abbrev=4 initial
+:100644 100644 35d2 9929 M	dir/sub
+:100644 100644 01e7 10a8 M	file0
+:000000 100644 0000 b1e6 A	file1
+:100644 000000 01e7 0000 D	file2
+$
diff --git a/t/t4013/diff.noellipses-diff_--raw_initial b/t/t4013/diff.noellipses-diff_--raw_initial
new file mode 100644
index 000000000000..0175bfb28166
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff_--raw_initial
@@ -0,0 +1,6 @@
+$ git diff --raw initial
+:100644 100644 35d242b 992913c M	dir/sub
+:100644 100644 01e79c3 10a8a9f M	file0
+:000000 100644 0000000 b1e6722 A	file1
+:100644 000000 01e79c3 0000000 D	file2
+$
diff --git a/t/t4013/diff.noellipses-show_--patch-with-raw_side b/t/t4013/diff.noellipses-show_--patch-with-raw_side
new file mode 100644
index 000000000000..32fed3d5764b
--- /dev/null
+++ b/t/t4013/diff.noellipses-show_--patch-with-raw_side
@@ -0,0 +1,42 @@
+$ git show --patch-with-raw side
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+:100644 100644 35d242b 7289e35 M	dir/sub
+:100644 100644 01e79c3 f4615da M	file0
+:000000 100644 0000000 7289e35 A	file3
+
+diff --git a/dir/sub b/dir/sub
+index 35d242b..7289e35 100644
+--- a/dir/sub
++++ b/dir/sub
+@@ -1,2 +1,4 @@
+ A
+ B
++1
++2
+diff --git a/file0 b/file0
+index 01e79c3..f4615da 100644
+--- a/file0
++++ b/file0
+@@ -1,3 +1,6 @@
+ 1
+ 2
+ 3
++A
++B
++C
+diff --git a/file3 b/file3
+new file mode 100644
+index 0000000..7289e35
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,4 @@
++A
++B
++1
++2
+$
diff --git a/t/t4013/diff.noellipses-whatchanged_--root_master b/t/t4013/diff.noellipses-whatchanged_--root_master
new file mode 100644
index 000000000000..c2cfd4e72927
--- /dev/null
+++ b/t/t4013/diff.noellipses-whatchanged_--root_master
@@ -0,0 +1,42 @@
+$ git whatchanged --root master
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+:100644 100644 35d242b 7289e35 M	dir/sub
+:100644 100644 01e79c3 f4615da M	file0
+:000000 100644 0000000 7289e35 A	file3
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+:100644 100644 8422d40 cead32e M	dir/sub
+:000000 100644 0000000 b1e6722 A	file1
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+:100644 100644 35d242b 8422d40 M	dir/sub
+:100644 100644 01e79c3 b414108 M	file0
+:100644 000000 01e79c3 0000000 D	file2
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+
+:000000 100644 0000000 35d242b A	dir/sub
+:000000 100644 0000000 01e79c3 A	file0
+:000000 100644 0000000 01e79c3 A	file2
+$
diff --git a/t/t4013/diff.noellipses-whatchanged_-SF_master b/t/t4013/diff.noellipses-whatchanged_-SF_master
new file mode 100644
index 000000000000..b36ce5886e0e
--- /dev/null
+++ b/t/t4013/diff.noellipses-whatchanged_-SF_master
@@ -0,0 +1,9 @@
+$ git whatchanged -SF master
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+:100644 100644 8422d40 cead32e M	dir/sub
+$
diff --git a/t/t4013/diff.noellipses-whatchanged_master b/t/t4013/diff.noellipses-whatchanged_master
new file mode 100644
index 000000000000..55e500f2edbe
--- /dev/null
+++ b/t/t4013/diff.noellipses-whatchanged_master
@@ -0,0 +1,32 @@
+$ git whatchanged master
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+:100644 100644 35d242b 7289e35 M	dir/sub
+:100644 100644 01e79c3 f4615da M	file0
+:000000 100644 0000000 7289e35 A	file3
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+:100644 100644 8422d40 cead32e M	dir/sub
+:000000 100644 0000000 b1e6722 A	file1
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+:100644 100644 35d242b 8422d40 M	dir/sub
+:100644 100644 01e79c3 b414108 M	file0
+:100644 000000 01e79c3 0000000 D	file2
+$
-- 
2.13.6

Re: [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> +# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS
> +
> +# The first detach operation is more chatty than the following ones.
> +cat > 1st_detach <<'EOF'
> +Note: checking out 'HEAD^'.
> +
> +You are in 'detached HEAD' state. You can look around, make experimental
> +changes and commit them, and you can discard any commits you make in this
> +state without impacting any branches by performing another checkout.
> +
> +If you want to create a new branch to retain commits you create, you may
> +do so (now or later) by using -b with the checkout command again. Example:
> +
> +  git checkout -b <new-branch-name>
> +
> +HEAD is now at 7c7cd714e262 three
> +EOF
> +# The remaining ones just show info about previous and current HEADs.
> +cat > 2nd_detach <<'EOF'
> +Previous HEAD position was 7c7cd714e262 three
> +HEAD is now at 139b20d8e6c5 two
> +EOF
> +cat > 3rd_detach <<'EOF'
> +Previous HEAD position was 139b20d8e6c5 two
> +HEAD is now at d79ce1670bdc one
> +EOF

It is preferrable to have all of the above inside the
test_expect_success block that uses them.  

Also lose the SP between redirection operator and its target
filename, i.e.

	command >file

not

	command > file

> +test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' '
> +	reset && check_not_detached && unset GIT_PRINT_SHA1_ELLIPSIS &&
> +
> +	# Various ways of *not* asking for ellipses
> +
> +	unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1> actual 2>&1 &&

Use sane_unset from t/test-lib-functions.sh instead, unless you are
absolutely sure that the variable you are unsetting _is_ set at this
point.

> +	check_detached &&
> +	test_cmp 1st_detach actual && unset GIT_PRINT_SHA1_ELLIPSIS &&

Is the output we are grabbing with check_detached from the command
internationalized?  If so, the comparison should be done with
test_i18ncmp (otherwise, the test will break under the "poisoned
gettext" build).

Thanks.

Re: [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot").

[Junio C Hamano]

I found a few questionable pieces in 4/7 but other than that the
whole series is very nicely done.

Will queue; thanks.

Re: [PATCH v6 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes".

Correct documentation with respect to what describe_detached_head prints
when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above.

Add tests for the old and new behaviour.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account
 Documentation/user-manual.txt |   2 +-
 builtin/checkout.c            |  10 +++-
 t/t2020-checkout-detach.sh    | 114 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 497e82e88dd0..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3faae382de4f..b0499542158f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,10 +400,16 @@ static void show_local_changes(struct object *head,
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (print_sha1_ellipsis()) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index fbb4ee9bb42d..3a37e07531ce 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -186,4 +186,118 @@ test_expect_success 'no advice given for explicit detached head state' '
 	test_cmp expect.no-advice actual
 '
 
+# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format)
+test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' "
+
+	# The first detach operation is more chatty than the following ones.
+	cat 1>1st_detach <<'EOF' &&
+Note: checking out 'HEAD^'.
+
+You are in 'detached HEAD' state. You can look around, make experimental
+changes and commit them, and you can discard any commits you make in this
+state without impacting any branches by performing another checkout.
+
+If you want to create a new branch to retain commits you create, you may
+do so (now or later) by using -b with the checkout command again. Example:
+
+  git checkout -b <new-branch-name>
+
+HEAD is now at 7c7cd714e262 three
+EOF
+
+	# The remaining ones just show info about previous and current HEADs.
+	cat 1>2nd_detach <<'EOF' &&
+Previous HEAD position was 7c7cd714e262 three
+HEAD is now at 139b20d8e6c5 two
+EOF
+
+	cat 1>3rd_detach <<'EOF' &&
+Previous HEAD position was 139b20d8e6c5 two
+HEAD is now at d79ce1670bdc one
+EOF
+
+	reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Various ways of *not* asking for ellipses
+
+	sane_unset GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# We only have four commits, but we can re-use them
+	reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Make no mention of the env var at all
+	git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=nein && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	true
+"
+
+# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format)
+test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' "
+
+	# The first detach operation is more chatty than the following ones.
+	cat 1>1st_detach <<'EOF' &&
+Note: checking out 'HEAD^'.
+
+You are in 'detached HEAD' state. You can look around, make experimental
+changes and commit them, and you can discard any commits you make in this
+state without impacting any branches by performing another checkout.
+
+If you want to create a new branch to retain commits you create, you may
+do so (now or later) by using -b with the checkout command again. Example:
+
+  git checkout -b <new-branch-name>
+
+HEAD is now at 7c7cd714e262... three
+EOF
+
+	# The remaining ones just show info about previous and current HEADs.
+	cat 1>2nd_detach <<'EOF' &&
+Previous HEAD position was 7c7cd714e262... three
+HEAD is now at 139b20d8e6c5... two
+EOF
+
+	cat 1>3rd_detach <<'EOF' &&
+Previous HEAD position was 139b20d8e6c5... two
+HEAD is now at d79ce1670bdc... one
+EOF
+
+	reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# Various ways of asking for ellipses...
+
+	GIT_PRINT_SHA1_ELLIPSIS="yes" && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	true
+"
+
 test_done
-- 
2.13.6

Re: [PATCH v6 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account

>  t/t2020-checkout-detach.sh    | 114 ++++++++++++++++++++++++++++++++++++++++++
> ...

Thanks; with this one replaced, I'd expect that poisoned gettext
test to pass now.

I saw some style issues, so I'll queue a tentative fix-up on top.

> +	# The first detach operation is more chatty than the following ones.
> +	cat 1>1st_detach <<'EOF' &&
> +Note: checking out 'HEAD^'.
> +
> +You are in 'detached HEAD' state. You can look around, make experimental
> +changes and commit them, and you can discard any commits you make in this
> +state without impacting any branches by performing another checkout.
> +
> +If you want to create a new branch to retain commits you create, you may
> +do so (now or later) by using -b with the checkout command again. Example:
> +
> +  git checkout -b <new-branch-name>
> +
> +HEAD is now at 7c7cd714e262 three
> +EOF

It looks somewhat strange to explicitly say 1> when redirecting the
standard output.  Also we prefer to indent our here-doc to the same
depth as commands by using "<<-", i.e.

	cat >1st_detach <<-'EOF' &&
	Note: checking out 'HEAD^'.
	...
	EOF

> +	reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&

It also was a bit irritating to see multiple commands form an
overlong single line, like this one.

> +	test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
> +
> +	GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
> +	check_detached &&
> +	test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&

This part uses "set and export only for a single invocation of git",
and because the variable is unset at the end of the previous step
after 1st_detach and actual are compared, this unset at the end
feels redundant.

> +	GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&

But now this part sets and exports the variable for the remainder of
the script (until it is unset).

Is the use of these two styles, i.e.

	VAR=value && export VAR && git -c core.abbrev=12 subcmd
	VAR=value git -c core.abbrev=12 subcmd

intended?  If so for what purpose?  It's not like we are testing if
the shell implements environment variables correctly, so I'd expect
that the result would be easier to follow if it stuck to a single
style and used it consistently.

Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

We do not want an ellipsis displayed following an (abbreviated) SHA-1
value.

The days when this was necessary to indicate the truncation to
lower-level Git commands and/or the user are bygone.

However, to ease the transition, the ellipsis will still be printed if
the user sets the environment variable GIT_PRINT_SHA1_ELLIPSIS to "yes".

Correct documentation with respect to what describe_detached_head prints
when GIT_PRINT_SHA1_ELLIPSIS is not set as indicated above.

Add tests for the old and new behaviour.

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
---
v2: rename patch series & focus on removal of ellipses
v3: env var instead of config option, use one-line comments where appropriate, preserve indent level
v4: improve env var handling (rename, helper func to query, docu)
v5: rewrite series to take Junio's comments in <xmqqd145k9td.fsf@gitster.mtv.corp.google.com> aboard
v6: polish to take Junio's comments from <xmqqshcqmoe7.fsf@gitster.mtv.corp.google.com> into account
v7: fix style issues (redirection, here-dox, long lines, setting/exporting/unsetting of env var): cf. <xmqq8tehjh6f.fsf@gitster.mtv.corp.google.com>
 Documentation/user-manual.txt |   2 +-
 builtin/checkout.c            |  10 +++-
 t/t2020-checkout-detach.sh    | 123 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 497e82e88dd0..eff78902742a 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -319,7 +319,7 @@ do so (now or later) by using -b with the checkout command again. Example:
 
   git checkout -b new_branch_name
 
-HEAD is now at 427abfa... Linux v2.6.17
+HEAD is now at 427abfa Linux v2.6.17
 ------------------------------------------------
 
 The HEAD then refers to the SHA-1 of the commit instead of to a branch,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3faae382de4f..b0499542158f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,10 +400,16 @@ static void show_local_changes(struct object *head,
 static void describe_detached_head(const char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+
 	if (!parse_commit(commit))
 		pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
-	fprintf(stderr, "%s %s... %s\n", msg,
-		find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	if (print_sha1_ellipsis()) {
+		fprintf(stderr, "%s %s... %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	} else {
+		fprintf(stderr, "%s %s %s\n", msg,
+			find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV), sb.buf);
+	}
 	strbuf_release(&sb);
 }
 
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index fbb4ee9bb42d..e8e284cf9c04 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -186,4 +186,127 @@ test_expect_success 'no advice given for explicit detached head state' '
 	test_cmp expect.no-advice actual
 '
 
+# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format)
+test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' "
+
+	# The first detach operation is more chatty than the following ones.
+	cat >1st_detach <<-'EOF' &&
+	Note: checking out 'HEAD^'.
+
+	You are in 'detached HEAD' state. You can look around, make experimental
+	changes and commit them, and you can discard any commits you make in this
+	state without impacting any branches by performing another checkout.
+
+	If you want to create a new branch to retain commits you create, you may
+	do so (now or later) by using -b with the checkout command again. Example:
+
+	  git checkout -b <new-branch-name>
+
+	HEAD is now at 7c7cd714e262 three
+	EOF
+
+	# The remaining ones just show info about previous and current HEADs.
+	cat >2nd_detach <<-'EOF' &&
+	Previous HEAD position was 7c7cd714e262 three
+	HEAD is now at 139b20d8e6c5 two
+	EOF
+
+	cat >3rd_detach <<-'EOF' &&
+	Previous HEAD position was 139b20d8e6c5 two
+	HEAD is now at d79ce1670bdc one
+	EOF
+
+	reset &&
+	check_not_detached &&
+
+	# Various ways of *not* asking for ellipses
+
+	sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS= git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual &&
+
+	sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
+
+	# We only have four commits, but we can re-use them
+	reset &&
+	check_not_detached &&
+
+	# Make no mention of the env var at all
+	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='nope' &&
+	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=nein &&
+	git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual &&
+
+	true
+"
+
+# Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format)
+test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' "
+
+	# The first detach operation is more chatty than the following ones.
+	cat >1st_detach <<-'EOF' &&
+	Note: checking out 'HEAD^'.
+	
+	You are in 'detached HEAD' state. You can look around, make experimental
+	changes and commit them, and you can discard any commits you make in this
+	state without impacting any branches by performing another checkout.
+	
+	If you want to create a new branch to retain commits you create, you may
+	do so (now or later) by using -b with the checkout command again. Example:
+	
+	  git checkout -b <new-branch-name>
+	
+	HEAD is now at 7c7cd714e262... three
+	EOF
+
+	# The remaining ones just show info about previous and current HEADs.
+	cat >2nd_detach <<-'EOF' &&
+	Previous HEAD position was 7c7cd714e262... three
+	HEAD is now at 139b20d8e6c5... two
+	EOF
+
+	cat >3rd_detach <<-'EOF' &&
+	Previous HEAD position was 139b20d8e6c5... two
+	HEAD is now at d79ce1670bdc... one
+	EOF
+
+	reset &&
+	check_not_detached &&
+
+	# Various ways of asking for ellipses...
+	# The user can just use any kind of quoting (including none).
+
+	GIT_PRINT_SHA1_ELLIPSIS="yes" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 1st_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 2nd_detach actual &&
+
+	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	check_detached &&
+	test_i18ncmp 3rd_detach actual &&
+
+	true
+"
+
 test_done
-- 
2.13.6

Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Junio C Hamano]

Ann T Ropea <bedhanger@gmx.de> writes:

> v7: fix style issues (redirection, here-dox, long lines, setting/exporting/unsetting of env var): cf. <xmqq8tehjh6f.fsf@gitster.mtv.corp.google.com>

Thanks.  I'd say this is polished enough already.  Let's stop
rerolling, wait for a few days for others to give final comments and
merge it to 'next'.

I've however queued the following on top of the entire series.  I do
not mind squashing it into this step, though.

-- >8 --
Subject: [PATCH] t2020: test variations that matter

Because our test suite is not about validating the working of the
shell, it is pointless to test variations of how a literal string
'yes' is quoted when assigned to an environment variable.

Instead, test various ways to spell 'yes' (we use strcasecmp() so
uppercased and capitalized variant should work just like 'yes'
spelled in all lowercase) and make sure we take them as 'yes'.  That
is more relevant in testing Git.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t2020-checkout-detach.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 9464069d15..bb4f2e0c63 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -294,15 +294,15 @@ test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked
 	# Various ways of asking for ellipses...
 	# The user can just use any kind of quoting (including none).
 
-	GIT_PRINT_SHA1_ELLIPSIS="yes" git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_i18ncmp 1st_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS='yes' git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_PRINT_SHA1_ELLIPSIS=Yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_i18ncmp 2nd_detach actual &&
 
-	GIT_PRINT_SHA1_ELLIPSIS=yes git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
+	GIT_PRINT_SHA1_ELLIPSIS=YES git -c 'core.abbrev=12' checkout HEAD^ >actual 2>&1 &&
 	check_detached &&
 	test_i18ncmp 3rd_detach actual &&
 
-- 
2.15.1-465-g070199a3f5

Re: [PATCH v7 4/7] checkout: describe_detached_head: remove ellipsis after committish

[Ann T Ropea]

Junio C Hamano <gitster@pobox.com> writes:

> I've however queued the following on top of the entire series.  I do
> not mind squashing it into this step, though.

> Subject: [PATCH] t2020: test variations that matter

That sounds and looks fine to me.  I'm going to do this on my
side (getting it from 'pu' once the squashing is done).

Thanks.