Jeff King <peff@xxxxxxxx> writes:
> We could also do it as:
>
> ref: refs/heads/master HEAD
>
> which matches the symref format itself. I guess that doesn't really
> matter here, but somehow it seems more aesthetically pleasing to me.
Yes, I think this should be the way to go.
> The output would look a lot nicer for humans if we right-padded the
> symref destination to match the 40-hex that is on all the other lines
> (so all of the refnames line up). But that makes machine-parsing a lot
> harder. We could do something clever with isatty(1), but I don't think
> it's worth the effort.
And the target can be longer than 40-hex. Don't play games with
padding.
>> +test_expect_success 'ls-remote with symrefs and refs combined' '
>> + cat >expect <<-EOF &&
>> + symref: refs/heads/master HEAD
>> + 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/master
>> + 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/remotes/origin/HEAD
>> + 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/remotes/origin/master
>> + 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/tags/mark
>> + EOF
>
> I expected there to be a:
>
> 1bd44cb9d13204b0fe1958db0082f5028a16eb3a HEAD
>
> line.
Yes, there should be, as I would imagine the most natural
interpretation of "--symrefs" by end users would be "show me ALSO
the symref information.", not "show me ONLY the symref information"
(if it were the latter we woudln't be seeing refs/tags/mark in the
above output).
I also suspect that this part of the patch is wrong (or at least
misleading):
@@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
+ if (symrefs && ref->symref)
+ printf("symref: %s %s\n", ref->symref, ref->name);
+ if (symrefs && !flags)
+ continue;
if (!check_ref_type(ref, flags))
continue;
if (!tail_match(pattern, ref->name))
It looks wrong that the usual filtering with check_ref_type() and
tail_match() are bypassed for symbolic refs. Even though the server
side currently feeds reflog information for only "HEAD", the code
should be prepared to do the sane thing in a future in which that
server-side limitation is corrected, and allow the user to ask
ls-remote $there --symref refs/remotes/origin/HEAD
to learn the information on one specific ref, without having to see
the primary branch of $there repository, for example.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html