Diane Gasselin <diane.gasselin@xxxxxxxxxxxxxxx> writes:
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -60,6 +60,92 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
> }
>
> /*
> + * add error messages on path <path> and action <action>
> + * corresponding to the type <e> with the message <msg>
> + * indicating if it should be display in porcelain or not
> + */
> +static int add_rejected_path(struct unpack_trees_options *o,
> + enum unpack_trees_error e,
> + const char *path,
> + const char *action,
> + int porcelain,
> + const char *msg)
> +{
> + struct rejected_paths_list *newentry;
> + struct rejected_paths **rp;
> + /*
> + * simply display the given error message if in plumbing mode
> + */
> + if (!porcelain)
> + o->show_all_errors = 0;
> + if (!o->show_all_errors)
> + return error(msg, path, action);
I don't fully understand what you're doing with show_all_errors and
porcelain here. From the caller, "porcelain" is true iff the
corresponding error message has been set in o. But if you can infer
whether you're in porcelain from the error messages, why do you need
show_all_errors in addition?
> static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o)
> {
> - return error(ERRORMSG(o, would_overwrite), ce->name);
> + return add_rejected_path(o, would_overwrite, ce->name, NULL,
> + (o && (o)->msgs.would_overwrite),
Parenthesis around (o) are distracting and useless. I guess you
copy-pasted from a macro (for which parentheses should definitely be
used in case the macro is called on an arbitrary expression).
> @@ -874,8 +964,16 @@ static int verify_uptodate_1(struct cache_entry *ce,
> }
> if (errno == ENOENT)
> return 0;
> - return o->gently ? -1 :
> - error(error_msg, ce->name);
> + if (error == sparse_not_uptodate_file)
> + return o->gently ? -1 :
> + add_rejected_path(o, sparse_not_uptodate_file, ce->name, NULL,
> + (o && (o)->msgs.sparse_not_uptodate_file),
> + ERRORMSG(o, sparse_not_uptodate_file));
> + else
> + return o->gently ? -1 :
> + add_rejected_path(o, not_uptodate_file, ce->name, NULL,
> + (o && (o)->msgs.not_uptodate_file),
> + ERRORMSG(o, not_uptodate_file));
> }
Isn't that a complex way of saying
int porcelain;
if (error == sparse_not_uptodate_file)
porcelain = o && o->msgs.sparse_not_uptodate_file;
else
porcelain = o && o->msgs.not_uptodate_file;
return o->gently ? -1 :
add_rejected_path(o, error, ce->name, NULL,
porcelain, ERRORMSG(o, error));
?
Also, I'm not sure I understand why you're attaching the error message
string to each rejected_paths entry. Wouldn't it be more sensible to
use o->msg in display_error_msgs() instead?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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