(I've snipped the comments that I don't dispute or need to comment on) On Thursday 28 April 2011, Junio C Hamano wrote: > The error output shows "error:" followed by "warning:", which looked > somewhat questionable. Perhaps allow a pointer to a structure be passed > in to describe the nature of a breakage to parse_dirstat_params()? Not sure what you mean here. You want the caller to supply a string_list, to which parse_dirstat_params() appends error messages, and then the caller determines how to display those error messages to the user after parse_dirstat_params() has returned? I'd rather go the simpler way, and simply turn the first "error:" into a "warning:". In other words, parse_dirstat_params() should only output "warning:", and then it's up to the caller whether to follow up with another "warning:" (in the diff.dirstat case), or a "fatal:" (in the --dirstat case). Otherwise, I agree with all your comments, and the next re-roll will be updated accordingly. ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- 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