From: "Rubén Justo" <rjusto@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Eric Sunshine <sunshine@sunshineco.com>
Cc: "Git List" <git@vger.kernel.org>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v4] branch: support for shortcuts like @{-1}, completed
Date: Mon, 10 Oct 2022 00:27:57 +0200 [thread overview]
Message-ID: <afec5288-7551-ff42-6dd8-899b5116394f@gmail.com> (raw)
In-Reply-To: <xmqq5ygs7pxg.fsf@gitster.g>
For me, the most value here is in the expressiveness, writing and reading
a simple test.
git config branch.desc-branch.description >actual &&
test_cmp actual "Branch description\n\n"
vs
git config branch.desc-branch.description >actual &&
printf "Branch description\n\n" >expect &&
test_cmp expect actual
vs
printf "Branch description\n\n" >expect &&
...
git config branch.desc-branch.description >actual &&
test_cmp expect actual
vs (oops)
printf "Branch description\n\n" >expect &&
...
git config branch.desc-branch.description >actual &&
printf "New branch description\n\n" >expect_ &&
test_cmp expect actual
Less lines, less files and less error prone are a plus.
On 9/10/22 8:46, Eric Sunshine wrote:
> My knee-jerk reaction is that the form which takes a string as
> argument would hardly be used, thus an unnecessary complication. The
A quick overview over a not much elaborated search:
git grep -B 3 'test_cmp expect' | grep '\(echo\|cat\).*expect'
gives many results that imo should be beneficial of this expressiveness.
> form which accepts stdin could even be retrofitted onto the existing
> test_cmp, thus you don't even need to invent a new function name. A
Nice. Maybe even the printf form with a "test ! -f", some risky though.
> different approach would be to introduce a function `test_stdout`
> which both accepts a command to run, as well as the "expected" text on
> stdin with which to compare the output of the command; i.e. a
> combination of the existing `test_stdout_line_count` in
> t/test-lib-functions.sh and your proposed function above.
Sounds good to me but maybe this goes in the direction of "inducing
bad usages", and the tests to cover starts to be not so simple...
On 9/10/22 21:33, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> That said, I'm not yet seeing all that much added value in such a
>> function; at most, it seems to save only a single line of code, and
>> it's not as if the code it's replacing was doing anything complicated
>> or hard to grok.
>
> I share the sentiment. Leaving the result that was used in comparison
> in file(s) also helps debugging.
>
And if this is fulfilled with a drop a file on failure? That resolves the
'-' in the diff output result too.
Thank you.
next prev parent reply other threads:[~2022-10-09 23:40 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-05 14:34 [PATCH 0/2] branch: support for at-refs like @{-1} in --edit-description, --set-upstream-to and --unset-upstream Rubén Justo via GitGitGadget
2022-09-05 14:34 ` [PATCH 1/2] branch: refactor edit_description command switch case Rubén Justo via GitGitGadget
2022-09-05 14:34 ` [PATCH 2/2] branch: support for at-refs like @{-1} Rubén Justo via GitGitGadget
2022-09-07 9:45 ` [PATCH v2 0/2] branch: support for shortcuts like @{-1}, completed Rubén Justo
2022-09-07 9:52 ` [PATCH v2 1/2] branch: refactor "edit_description" code path Rubén Justo
2022-09-07 20:25 ` Junio C Hamano
2022-09-07 21:24 ` Rubén Justo
2022-09-08 4:32 ` Rubén Justo
2022-09-07 9:53 ` [PATCH v2 2/2] branch: support for shortcuts like @{-1} completed Rubén Justo
2022-09-08 4:47 ` [PATCH v3 0/2] branch: support for shortcuts like @{-1}, completed Rubén Justo
2022-09-08 4:51 ` [PATCH v3 1/2] branch: refactor "edit_description" code path Rubén Justo
2022-09-08 20:57 ` [PATCH] branch: error codes for "edit_description" Rubén Justo
2022-09-08 21:45 ` Junio C Hamano
2022-09-08 4:53 ` [PATCH v3 2/2] branch: support for shortcuts like @{-1} completed Rubén Justo
2022-10-08 1:00 ` [PATCH v4] branch: support for shortcuts like @{-1}, completed Rubén Justo
2022-10-08 3:17 ` Eric Sunshine
2022-10-08 4:23 ` Junio C Hamano
2022-10-08 7:07 ` Rubén Justo
2022-10-08 7:23 ` Eric Sunshine
2022-10-08 9:12 ` Rubén Justo
2022-10-08 17:10 ` Eric Sunshine
2022-10-08 17:46 ` Junio C Hamano
2022-10-08 20:48 ` Rubén Justo
2022-10-08 23:28 ` Rubén Justo
2022-10-09 6:46 ` Eric Sunshine
2022-10-09 19:33 ` Junio C Hamano
2022-10-09 22:27 ` Rubén Justo [this message]
2022-10-08 4:17 ` Junio C Hamano
2022-10-08 9:04 ` Rubén Justo
2022-10-08 22:32 ` [PATCH v5] " Rubén Justo
2022-10-09 5:37 ` Junio C Hamano
2022-10-09 19:11 ` Junio C Hamano
2022-10-09 21:26 ` Rubén Justo
2022-10-10 0:38 ` Junio C Hamano
2022-10-10 6:05 ` Rubén Justo
2022-10-10 16:55 ` Junio C Hamano
2022-10-10 18:08 ` Rubén Justo
2022-10-10 23:24 ` [PATCH v6] " Rubén Justo
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=afec5288-7551-ff42-6dd8-899b5116394f@gmail.com \
--to=rjusto@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sunshine@sunshineco.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).