On Wed, Aug 14, 2019 at 09:57:50AM +0000, Paolo Pettinato (ppettina) wrote: > Could not access submodule 'sm' # fails, plus no newline here :P! This part seems easy enough to fix. -- >8 -- Subject: get_next_submodule(): format error string as an error The run_processes_parallel() interface passes its callback functions an "err" strbuf in which they can accumulate errors. However, this differs from our usual "err" strbufs in that the result is not simply passed to error(), like: if (frob_repo(&err) < 0) error("frobnication failed: %s", err.buf); Instead, we append the error buffer as-is to a buffer collecting the sub-process stderr, adding neither a prefix nor a trailing newline. This gives callbacks more flexibility (e.g., get_next_submodule() adds its own "Fetching submodule foo" informational lines), but it means they're also responsible for formatting any errors themselves. We forgot to do so in the single error message in get_next_submodule(), meaning that it was output without a trailing newline. While we're fixing that, let's also give it the usual "error:" prefix and downcase the start of the message. We can't use error() here, because it always outputs directly to stderr. Looking at other users of run_processes_parallel(), there are a few similar messages in update_clone_task_finished(). But those sites do correctly add a newline (they don't use an "error" prefix, but it doesn't make as much sense there). Signed-off-by: Jeff King <peff@xxxxxxxx> --- submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodule.c b/submodule.c index 0f199c5137..a5ba57ac36 100644 --- a/submodule.c +++ b/submodule.c @@ -1478,7 +1478,7 @@ static int get_next_submodule(struct child_process *cp, !is_empty_dir(ce->name)) { spf->result = 1; strbuf_addf(err, - _("Could not access submodule '%s'"), + _("error: could not access submodule '%s'\n"), ce->name); } } -- 2.23.0.rc2.479.gbd16c8906f