On Wed, Jul 31, 2024 at 7:29 PM Junio C Hamano [off-list ref] wrote:

Christian Couder [off-list ref] writes:

quoted

We often have to split strings at some specified terminator character.
The strbuf_split*() functions, that we can use for this purpose,
return substrings that include the terminator character, so we often
need to remove that character.

When it is a whitespace, newline or directory separator, the
terminator character can easily be removed using an existing triming
function like strbuf_rtrim(), strbuf_trim_trailing_newline() or
strbuf_trim_trailing_dir_sep(). There is no function to remove that
character when it's not one of those characters though.

OK.

quoted

Let's introduce a new strbuf_trim_trailing_ch() function that can be
used to remove any trailing character, and let's refactor existing code
that manually removed trailing characters using this new function.

It is disappointing that this new one is not adequate to rewrite any
of the existing strbuf_trim* functions in terms of it, but that's
probably OK.

Yeah, I took a look at that but thought it wasn't worth trying to
unify the trim functions as they each have quite specific code and
requirements.

At least this one we have two existing callers, but
makes me wonder if these callers are doing sensible things in the
first place.  After trimming trailing commas, there may be trailing
newlines to be trimmed, and then again whitespaces around the whole
thing may need to be trimmed---what kind of input is that?  The
value has to be " junk \n\n,,,", but " junk, \n\n, " will only
become "junk, \n\n," without further cleaned up, and it is very
dubious how that is useful.

But that is not an issue this patch introduces ;-)