Re: [PATCHv5 7/7] Improve error handling when parsing dirstat parameters

(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, [off-list ref]
www.herland.net