Junio C Hamano wrote: > Jim Meyering <jim@xxxxxxxxxxxx> writes: > >> What do you think about replacing those two append-if-needed two-liners: >> >> if (buffer2.len && buffer2.buf[buffer2.len - 1] != '/') >> strbuf_addch(&buffer2, '/'); >> >> by something that readably encapsulates the idiom: >> >> strbuf_append_if_absent (&buffer2, '/'); >> >> (though the name isn't particularly apt, because you might >> take "absent" to mean "not anywhere in the string," so maybe >> strbuf_append_if_not_already_at_end (ugly) or >> strbuf_append_uniq >> ) > > I am not good at names, but strbuf_terminate_with(&buffer2, '/') > perhaps? Maybe, but it still doesn't evoke the conditional nature of don't-append-if-already-there the operation. i.e., one might wonder how it's different from "strbuf_append". How about one of these? strbuf_ensure_suffix // but might make you think suffix==more than 1 byte strbuf_ensure_last_byte // maybe? strbuf_ensure_last_byte_is // rather long, but apt >> There are several other uses that would benefit from such a transformation: >> To find the easy ones, I ran this: >> >> git grep -B1 "strbuf_addch.*'"|grep -A1 '!=' >> >> I've manually marked/separated the ones that don't apply. >> >> Note how only 2 of the 6 candidates ensure that length is positive >> before using ".len - 1": > > Yikes, that is embarrasing ;-) Knowing you/git, each is because the buffer is known to be non-empty. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html