All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philip Oakley" <philipoakley@iee.org>
To: "Junio C Hamano" <gitster@pobox.com>, "Ann T Ropea" <bedhanger@gmx.de>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Daniel Barkalow" <barkalow@iabervon.org>
Subject: Re: [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values
Date: Mon, 20 Nov 2017 12:25:34 -0000	[thread overview]
Message-ID: <5AE7AD53CF184A27BF8F484D415083D9@PhilipOakley> (raw)
In-Reply-To: xmqqzi7hlhkx.fsf@gitster.mtv.corp.google.com

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. 


  reply	other threads:[~2017-11-20 17:26 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea
2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea
2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea
2017-11-06  4:34   ` Junio C Hamano
2017-11-06  2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano
2017-11-07  0:30   ` Philip Oakley
2017-11-07  0:52 ` Junio C Hamano
2017-11-07  2:53 ` Ann T Ropea
2017-11-07 23:25   ` Philip Oakley
2017-11-08  1:59     ` Junio C Hamano
2017-11-09 23:15       ` Philip Oakley
2017-11-13 22:36         ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-14  3:08           ` Junio C Hamano
2017-11-19 17:38             ` Ann T Ropea
2017-11-20  1:48               ` Junio C Hamano
2017-11-19 18:41             ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-20  3:35               ` Junio C Hamano
2017-11-19 18:41             ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-19 19:11               ` Eric Sunshine
2017-11-19 18:41             ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-19 19:15               ` Eric Sunshine
2017-11-24 23:53                 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-25  5:01                   ` Junio C Hamano
2017-11-26  3:17                     ` Junio C Hamano
2017-11-26  3:19                       ` Junio C Hamano
2017-11-26  3:25                       ` Junio C Hamano
2017-12-03 21:27                       ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-12-04 16:52                         ` Junio C Hamano
2017-12-03 21:27                       ` [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 3/7] print_sha1_ellipsis: introduce helper Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-12-04 16:46                         ` Junio C Hamano
2017-12-04 23:13                           ` [PATCH v6 " Ann T Ropea
2017-12-05 16:03                             ` Junio C Hamano
2017-12-06  0:20                               ` [PATCH v7 " Ann T Ropea
2017-12-06 16:47                                 ` Junio C Hamano
2017-12-06 22:02                                   ` Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-19 18:41             ` [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-19 18:41             ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-20  4:06               ` Junio C Hamano
2017-11-20 12:25                 ` Philip Oakley [this message]
2017-11-22  5:53                   ` Junio C Hamano
2017-11-22 23:41                     ` Philip Oakley
2017-11-24  0:40                       ` Junio C Hamano
2017-11-13 22:36         ` [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-14  3:20           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5AE7AD53CF184A27BF8F484D415083D9@PhilipOakley \
    --to=philipoakley@iee.org \
    --cc=barkalow@iabervon.org \
    --cc=bedhanger@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.