Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
> Umm, since I'm currently working on another reroll that has pretty
> args to add notes refs for display...
>
> On Tuesday 23 February 2010 00:20:06 Junio C Hamano wrote:
>> Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
>> > + if (flags & NOTES_SHOW_HEADER_WITH_REF && t->ref) {
>> > + const char *ref = t->ref;
>> > + if (!strcmp(ref, GIT_NOTES_DEFAULT_REF)) {
>> > + strbuf_addstr(sb, "\nNotes:\n");
>> > + } else {
>> > + if (!prefixcmp(ref, "refs/"))
>> > + ref += 5;
>> > + if (!prefixcmp(ref, "notes/"))
>> > + ref += 6;
>> > + strbuf_addf(sb, "\nNotes (%s):\n", ref);
>> > + }
>> > + } else if (flags & (NOTES_SHOW_HEADER|NOTES_SHOW_HEADER_WITH_REF))
>> > strbuf_addstr(sb, "\nNotes:\n");
>>
>> It is not clear what the distinction between NOTES_SHOW_HEADER and
>> NOTES_SHOW_HEADER_WITH_REF. Does anybody still call this function with
>> NOTES_SHOW_HEADER alone without NOTES_SHOW_HEADER_WITH_REF?
>
> No.
Meaning nobody will go through the latter "Notes:\n" codepath? Then what
is that else clause for?
Perhaps I am not reading your code right in which case this part needs a
bit more commenting?
>> I expected to see "Notes:\n" regardless of the mode if the notes is coming
>> from the default refs/notes/commits tree, but it probably is better to say
>> "Notes (commits):\n" like your patch does.
>
> I special-cased GIT_NOTES_DEFAULT_REF (which is "refs/notes/commits")
> above *at your request* to not change the output in the default case.
> So which way do you want it?
I don't have strong preference anymore with the above code.
If some caller called the latter "always show Notes:" codepath, i.e.
SHOW_HEADER is given but without HEADER_WITH_REF, then my preference would
have been more clear. A caller that gives HEADER_WITH_REF clearly wants
to show more than one notes (perhaps the user used displayref mechanism)
and it would be better to always say which one in order to disambiguate,
and the special case would not be a good idea. A caller would not give
HEADER_WITH_REF when the user has refs/notes/commits hierarchy and nothing
else, and the user didn't ask for multiple notes trees shown; in such a
case we should show the good-old "Notes:".
But if everybody calls with HEADER_WITH_REF, no matter what the end user
preference is, then it becomes unclear what the right answer would be.
--
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