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

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

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(-)