Re: [PATCH v3 7/9] pretty: refactor `format_sanitized_subject()`

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

Hariom verma <hariom18599@gmail.com> writes:

>> Also, because neither LF or SP is a titlechar(), wouldn't the "if
>> r[i] is LF, replace it with SP" a no-op wrt what will be in sb at
>> the end?
>
> Maybe its better to directly replace LF with hyphen? [Instead of first
> replacing LF with SP and then replacing SP with '-'.]

Why do you think LF is so special?

Everything other than titlechar() including HT, '#', '*', SP is
treated in the same way as the body of that loop.  It does not
directly contribute to the final contents of sb, but just leaves
the marker in the variable "space" the fact that when adding the
next titlechar() to the resulting sb, we need a SP to wordbreak.

LF now happens to be in the set due to the way you extended the
function (it wasn't fed to this function by its sole caller), but
other than that, it is no more special than HT, SP or '*'.  And they
are not replaced with SP or replaced with '-'.

So it would be the most sensible to just drop 'if LF, replace it
with SP before doing anything else' you added.  The existing 'if
titlechar, add it to sb but if we saw non-title, add a SP before
doing so to wordbreak, and if not titlechar, just remember the fact
that we saw one' should work fine as-is without special casing LF at
all.

Or am I missing something subtle?