Re: [PATCH v3 7/9] pretty: refactor `format_sanitized_subject()`
- From
Hariom verma <hariom18599@gmail.com>
- Date
- Aug 20, 2020, 17:27 UTC
- Message-ID
- <CA+CkUQ8=ej=NuG=FjsE6oqT+n8YUpGeVVNyhhs3FZ7UwZ5pG_g@mail.gmail.com>
- In-Reply-To
- <xmqqimdevd4l.fsf@gitster.c.googlers.com>
Hi,
On Wed, Aug 19, 2020 at 9:31 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 31 quoted lines
> > 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?
You actually got it all right. Thanks for the insight.
Now, I got it. There is no need to give special attention to LF. I missed to see that `titlechar()` is already taking care of everything.
Thanks, Hariom