Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> writes:
> +give_advice:
> + advise(_("have you committed already?\n"
> + "try \"git %s --continue\""),
> + action == REPLAY_REVERT ? "revert" : "cherry-pick");
> + return error(_("there is nothing to skip"));
> +}
Two comments.
The places touched by patch 1/5 emitted the error followed by an
advice message; this new one breaks the pattern by giving the "hint:"
first and then error. Be consistent by swapping these two (and
return -1, as "error() that returns -1" will no longer be the last
thing executed in this function.
This one, and the in_progress_advice emitted from the patch 1/5, are
both bad in that they make calls to advise() without guarding it
with an advice.* configuration variable. This does not allow the
user to say say "I've learned this part of Git enough; do not tell
me verbosely."
Pick a random global variable that is defined near the top of
advice.c, and learn how they are set (to true by default, allowing
configuration to flip it off) and how they are used in order to
prevent a call to advise() getting made. Then mimick that to guard
these calls to advise().
Thanks.
