On Wed, Aug 1, 2018 at 5:41 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > Hmm, do we actually say things like "Error: blah"? I am not sure if > I like this strncasecmp all that much. this is for the remote end, so what we (git-core) says isn't all that relevant. The reason I put this here is that Gerrit has some messages that say "ERROR: .. " > >> + strbuf_addstr(dest, p->color); > >> + strbuf_add(dest, src, len); > >> + strbuf_addstr(dest, GIT_COLOR_RESET); > >> + n -= len; > >> + src += len; > >> + break; > >> + } > > > > So, despite the explanation in the commit message, this function isn't > > _generally_ highlighting keywords in the sideband. Instead, it is > > highlighting a keyword only if it finds it at the start of string > > (ignoring whitespace). Perhaps the commit message could be more clear > > about that. > > Sounds good. > > I didn't comment on other parts of your review posed as questions > (that require some digging and thinking), but I think they are all > worthwhile thing to think about. Sorry for being dense, but do you want me to send an updated patch or not based on your and Eric's comments or not? thanks, -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
