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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 23, 2021, 20:34 UTC
Message-ID: <xmqqzgpu7grv.fsf@gitster.g>
In-Reply-To: <CAFQ2z_Mct+KBZ3vO6udwqeiHYA8od8CGH_w5BO5LaidP-AYDsg@mail.gmail.com>

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

Show 10 quoted lines

> 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.