Re: [PATCHv5 7/7] Improve error handling when parsing dirstat parameters
- From
Johan Herland <johan@herland.net>
- Date
- Apr 28, 2011, 23:13 UTC
- Message-ID
- <201104290113.26481.johan@herland.net>
- In-Reply-To
- <7vhb9i43sl.fsf@alter.siamese.dyndns.org>
(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@herland.net> www.herland.net