Re: [PATCH v2 5/5] refs/debug: trim trailing LF from reflog message

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 29, 2021, 19:35 UTC
Message-ID: <xmqqwnkq69hq.fsf@gitster.g>
In-Reply-To: <xmqq1r2y7ork.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 27 quoted lines

> Han-Wen Nienhuys <hanwen@google.com> writes:
>
>> On Fri, Nov 26, 2021 at 9:16 AM Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> The API promises to have only LF, not CRLF, at the end, so
>>> strbuf_trim_trailing_newline() is a bit overkill (and if payload
>>> happened to end with CR, we would lose it).
>>
>> it would be best if there was a way to escape characters (ie. "\n" =>
>> "\\n"). Do we have a function for that?
>
> Mere escaping would not work in a backward compatible way, without a
> trick.  It was envisioned that we probably could encode *and* signal
> the fact that the message is encoded by appending a trailing SP at
> the end of the message.  See the log message of 523fa69c (reflog:
> cleanse messages in the refs.c layer, 2020-07-10) for details.
>
> Having said that, that is about introducing a whole new reflog
> message format (whose use is signalled by the trailing SP), and I
> would prefer it to happen
>
>  (1) after we integrate with reftable, and
>
>  (2) implemented as an option in the normalize_reflog_message()
>      function, so that no ref backends has to worry about it.
>
> outside this topic.

Sorry, I seem to have forgotten to give an answer to your question, and having warned sufficiently clearly that now is not the time to do it, I can safely do so. If we wanted to go the C-quote route, then the function we would use would be quote.c::quote_c_style().

Even though 523fa69c (reflog: cleanse messages in the refs.c layer, 2020-07-10) muses on use of urlencode, it shouldn't be taken as a sign that I prefer it over c-quoting, or vice versa for that matter.

It was merely an example encoding picked in the context of explaining that (1) I wasn't interested in keeping a multi-line message and spewing back as-is, and (2) but it is possible to loosen it in the future.