From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 29 Apr 2011 04:06:33 GMT
Subject: Re: [PATCHv5 7/7] Improve error handling when parsing dirstat parameters
Message-ID: <7vfwp13dme.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201104290113.26481.johan@herland.net>

Johan Herland <johan@herland.net> writes:

> 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?

A rough outline of what I had in mind was:

	struct dirstat_param_error {
        	enum {
                	ERR_DIRSTAT_PERCENT = 1,
                        ERR_DIRSTAT_UNKNOWN
		} kind;
                strbuf msg;
	};

	static int parse_dirstat_params(struct diff_options *options,
		        	const char *params,
				struct dirstat_param_error *errinfo)
	{
		while (...) {
                        ...
                        else if (isdigit(*p)) {
                                ...
                                if (end - p == p_len)
                                        options->dirstat_permille = permille;
                                else {
                                        errinfo->kind = ERR_DIRSTAT_PERCENT;
                                        strbuf_add(&errinfo->msg, p, p_len);
                                        ret = -1;
                                }
                        } else {
                                errinfo->kind = ERR_DIRSTAT_UNKNOWN;
                                strbuf_add(&errinfo->sb, p, p_len);
                                ret = -1;
                        }
                        p += p_len;
                        if (*p)
                                p++;
                }
                return ret;
	}

and then the caller can extract the information to format.

But you produce more than one one error messages, so a single errinfo
approach would not work.  Instead, we should be able to pass in the
pointer to a single strbuf errmsg, and accumulate the errors in it by
calling strbuf_addf() for the same effect.  The format string given to
strbuf_addf() may probably need to be marked with _().

The caller can then check errmsg->len to see if there was an error.