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

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

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

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

I was undecided between mentioning and not mentioning the variable
name "space" here.  On one hand, one _could_ argue that "space" is
used to remember we saw "space and the like" and if it were named
"seen_non_title_char", then such a confusion to treat LF so
specially might not have occurred.  But on the other hand, "space"
is what the variable exactly keeps track of; it is just the need for
space on the output side, i.e. we remember that "space needed before
the next output" with that variable.

I am inclined not to suggest renaming "space" at all, but it won't
be the end of the world if it were renamed to "need_space" (before
the next output), or "seen_nontitle".  If we were to actually
rename, I have moderately strong preference to the "need_space" over
"seen_nontitle", as it won't have to be renamed again when the logic
to require a space before the next output has to be updated to
include cases other than just "we saw a nontitle character".

Thanks.