On Wed, Jul 31, 2024 at 7:29 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Christian Couder <christian.couder@xxxxxxxxx> writes: > > > 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. > > > 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 ;-)
