From: phillip.wood123@gmail.com
To: John Cai <johncai86@gmail.com>, phillip.wood@dunelm.org.uk
Cc: "John Cai via GitGitGadget" <gitgitgadget@gmail.com>,
git@vger.kernel.org,
"Kristoffer Haugsbakk" <code@khaugsbakk.name>,
"Jeff King" <peff@peff.net>, "Patrick Steinhardt" <ps@pks.im>,
"Jean-Noël Avila" <avila.jn@gmail.com>
Subject: Re: [PATCH v2 3/3] show-ref: add --symbolic-name option
Date: Fri, 12 Apr 2024 10:37:16 +0100 [thread overview]
Message-ID: <4c0b6da2-56d3-4441-aaed-8d9a2d28d9c0@gmail.com> (raw)
In-Reply-To: <F53A2B72-D089-4C0A-92D0-E96EDD410A74@gmail.com>
Hi John
On 11/04/2024 20:57, John Cai wrote:
> On 9 Apr 2024, at 11:25, Phillip Wood wrote:
>>> +When using `--symbolic-name`, the output is in the format:
>>> +
>>> +-----------
>>> +<oid> SP <ref> SP <symbolic-name>
>>> +-----------
>>> +
>>> +For example,
>>> +
>>> +-----------------------------------------------------------------------------
>>> +$ git show-ref --symbolic-name
>>> +b75428bae1d090f60bdd4b67185f814bc8f0819d refs/heads/SYMBOLIC_REF ref:refs/heads/main
>>
>> Do we really need the "ref:" prefix? It is not specified above and I think anyone calling this would have to remove the prefix before they could use the value.
>
> I can see how it would be more ergonimic to just have the value without the
> "ref: " prefix. I kept it because that's how the refs are represented on disk
> and git-symbolic-ref prints them out with the "ref: " prefix.
>
> I don't have a strong preference, but I lean a bit towards keeping it consistent
> with the output of other commands.
I agree it is a good idea to keep things consistent, and dropping the
"ref:" prefix is consistent with other commands:
$ git symbolic-ref HEAD
refs/heads/rebase-fix-signoff
Best Wishes
Phillip
>
>>
>>
>>> +test_expect_success '--symbolic-name with symbolic ref' '
>>> + test_when_finished "git symbolic-ref -d refs/heads/SYMBOLIC_REF_A" &&
>>> + commit_oid=$(git rev-parse refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME) &&
>>> + cat >expect <<-EOF &&
>>> + $commit_oid refs/heads/SYMBOLIC_REF_A ref:refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>>> + EOF
>>> + git symbolic-ref refs/heads/SYMBOLIC_REF_A refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME &&
>>> + git show-ref --symbolic-name SYMBOLIC_REF_A >actual &&
>>> + test_cmp expect actual
>>> +'
>>
>> I think it would be nice to see a test along the lines of
>>
>> git symbolic-ref refs/symref-c refs/heads/master
>> git symbolic-ref refs/symref-b refs/symref-c &&
>> git symbolic-ref refs/symref-a refs/symref-b &&
>> git show-ref --symbolic-name refs/symref-a >actual &&
>> cat >expect <<\EOF &&
>> $(git show-ref -s --verify refs/heads/master) refs/heads/symref-a refs/heads/symref-b
>> EOF
>> test_cmp expect actual
>>
>> to show what this command is expected to return when there is a chain of symbolic references.
>
> good point, will add this in the next series.
>
>>
>> Best Wishes
>>
>> Phillip
>
> thanks for the review!
> John
next prev parent reply other threads:[~2024-04-12 9:37 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-04 22:51 [PATCH] show-ref: add --unresolved option John Cai via GitGitGadget
2024-03-04 23:23 ` Junio C Hamano
2024-03-05 20:56 ` John Cai
2024-03-05 21:29 ` Junio C Hamano
2024-03-05 15:30 ` Phillip Wood
2024-03-05 17:01 ` Kristoffer Haugsbakk
2024-03-06 0:33 ` Jeff King
2024-03-06 2:19 ` Junio C Hamano
2024-03-06 0:41 ` Jeff King
2024-03-06 7:31 ` Patrick Steinhardt
2024-03-06 7:51 ` Jeff King
2024-03-06 16:48 ` Junio C Hamano
2024-03-06 9:36 ` Jean-Noël Avila
2024-04-08 17:38 ` [PATCH v2 0/3] show-ref: add --symbolic-name option John Cai via GitGitGadget
2024-04-08 17:38 ` [PATCH v2 1/3] refs: keep track of unresolved reference value in iterator John Cai via GitGitGadget
2024-04-08 23:02 ` Junio C Hamano
2024-04-09 20:29 ` John Cai
2024-04-10 6:52 ` Patrick Steinhardt
2024-04-10 15:26 ` Junio C Hamano
2024-04-11 9:11 ` Patrick Steinhardt
2024-04-08 17:38 ` [PATCH v2 2/3] refs: add referent to each_repo_ref_fn John Cai via GitGitGadget
2024-04-08 17:38 ` [PATCH v2 3/3] show-ref: add --symbolic-name option John Cai via GitGitGadget
2024-04-09 15:25 ` Phillip Wood
2024-04-11 19:57 ` John Cai
2024-04-12 9:37 ` phillip.wood123 [this message]
2024-04-10 6:53 ` Patrick Steinhardt
2024-04-10 15:27 ` Junio C Hamano
2024-04-12 15:23 ` John Cai
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=4c0b6da2-56d3-4441-aaed-8d9a2d28d9c0@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=avila.jn@gmail.com \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johncai86@gmail.com \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=ps@pks.im \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).