Re: [PATCH 2/4] refs: trim newline from reflog message

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

Han-Wen Nienhuys <hanwen@google.com> writes:

> I'm talking about refs/refs-internal.h. It seems you want to add something like
>
> /* The ref backend should add a '\n' relative to the message supplied
> to the delete/symref/update functions. */
> typedef int for_each_reflog_ent_fn(struct ref_store *ref_store,
>                                    const char *refname,
>                                    each_reflog_ent_fn fn,
>                                    void *cb_data);
>
> ?

Sorry, I do not follow.  Doesn't the ref backend already ensure that
the message is not an incomplete line?  If you feed a single
complete line when updating, I do not think the backend should add
any extra LF relative to the given message:

    $ git update-ref -m 'creating a new branch manually
    ' refs/heads/newtest master
    $ git update-ref -m 'updating a new branch manually
    ' refs/heads/newtest master~1
    $ git reflog refs/heads/newtest
    4150a1677b refs/heads/newtest@{0}: updating a new branch manually
    5f439a0ecf refs/heads/newtest@{1}: updating the reference

I think what deserves such a comment more is the prototype for
each_reflog_ent_fn describing what the parameters to that callback
function, to help the callers of the iterator what to expect.  That
is the end-user facing part.

/*
 * Callback to process a reflog entry found by the iteration functions (see
 * below)
 */
typedef int each_reflog_ent_fn(
		struct object_id *old_oid, struct object_id *new_oid,
		const char *committer, timestamp_t timestamp,
		int tz, const char *msg, void *cb_data);

Currently it only says "Callback (see below)" but "below" has only
comments about the difference between refs_for_each_reflog_ent() and
refs_for_each_reflog_entreverse() functions, and does not talk about
what "committer" looks like (i.e. does it give user.name equivalent,
user.name plus <user.email>, or something else?), and what "msg"
looks like (i.e. if a multi-line message was given when a ref was
updated or created, do we get these lines intact? if it gets
mangled, how? if the original was a single-liner incomplete line, do
we lack the final LF?), and how tz is encoded.

I think the rule for "msg" is that:

   a multi-line message, or a message on a single incomplete-line,
   are normalized into a single complete line, and callback gets a
   single complete line.

Once these rules get specified tightly and fully, it is up to the
ref(log) backends how to implement the interface.  If files-backend
wants to keep LF at the end of the message when storing (so that it
can easily use it as record delimiter), it can do so and reuse the
LF at the end of the message as part of the msg parameter to the
callback.  If another backend wants to drop LF at the end of the
message when storing (to save space), it can do so as long as the
callback function gets the LF just like the other backend.