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? > 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 ;-) > > ------------------------------------ > builtin/branch.c- if (!buf.len || buf.buf[buf.len-1] != '\n') > builtin/branch.c: strbuf_addch(&buf, '\n'); > -- > builtin/fmt-merge-msg.c- if (out->buf[out->len - 1] != '\n') > builtin/fmt-merge-msg.c: strbuf_addch(out, '\n'); > -- > builtin/log.c- if (filename.buf[filename.len - 1] != '/') > builtin/log.c: strbuf_addch(&filename, '/'); > -- > builtin/notes.c- if (buf.buf[buf.len - 1] != '\n') > builtin/notes.c: strbuf_addch(&buf, '\n'); /* Make sure msg ends with newline */ > -- > refs.c- if (real_pattern.buf[real_pattern.len - 1] != '/') > refs.c: strbuf_addch(&real_pattern, '/'); > -- > strbuf.h- if (sb->len && sb->buf[sb->len - 1] != '\n') > strbuf.h: strbuf_addch(sb, '\n'); -- 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