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

[Junio C Hamano]

Junio C Hamano  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 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 00..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
+:00 100644  35d2 A dir/sub
+:00 100644  01e7 A file0
+:00 100644  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  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 "<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 
---
 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 | 

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

[Junio C Hamano]

Ann T Ropea  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 
> ---
> 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.