Re: [PATCH 1/4] test-ref-store: tweaks to for-each-reflog-ent format

[Junio C Hamano <gitster@pobox.com>]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Follow the reflog format more closely, so it can be used for comparing
> reflogs in tests without using inspecting files under .git/logs/

I find the above written in clear language that conveys what it
wants to say unclearly X-<.

I am guessing, from the patch text, that the callback function to
refs_for_each_reflog_ent() and its _reverse() variant takes a "msg"
parameter, which is expected to include a terminating LF at its end,
but some backends omits the LF, and this change is to work around it
by conditionally adding (or refraining from adding) LF in the
each_reflog() function that is used for testing.

Is that what is going on?  Or is it "even within a single backend,
sometimes we get message with terminating LF and sometimes without"?

In any case, it changes the output.  We used to spend two output
lines per record in tests for these two functions, but now we assume
we get one each line per one reflog entry.  Which may be OK as far
as these tests are concerned, but does this leave problems with the
real world callbacks?  Do the real callers to for_each_reflog_ent()
and for_each_reflog_ent_reverse() also now need to cater to this "we
sometimes get LF at the end, we sometimes don't" differences by
having a similar logic as we see in the updated each_reflog() in
this patch?

Shouldn't we be instead making sure that the callback functions get
the msg argument in a consistent way, whether it is with or without
terminating LF, instead of forcing the callers to cope with the
differences like this each_reflog() function does in this patch?

Thanks.