Re: [PATCH] refs.c: repack_without_refs may be called without error string buffer

[Jonathan Nieder <jrnieder@gmail.com>]

Stefan Beller wrote:

> If we don't pass in the error string buffer, we skip over all
> parts dealing with preparing error messages.

Please no.

We tried this with the ref transaction code.  When someone wants
to silence the message, it is cheap enough to do

	struct strbuf ignore = STRBUF_INIT;

	if (thing_that_can_fail_in_an_ignorable_way(..., &ignore)) {
		... handle the failure ...
	}

The extra lines of code make it obvious that the error message is
being dropped, which is a very good thing.  The extra work to format a
message in the error case is not so bad and can be mitigated if the
error is a common normal case by passing a flag to not consider it an
error.

Silently losing good diagnostic messages when err == NULL would have
the opposite effect: when there isn't a spare strbuf to put errors in
around, it would be tempting for people coding in a hurry to just pass
NULL, and to readers it would look at first glance like "oh, an
optional paramter was not passed and we are getting the good default
behavior".

This is not a theoretical concern --- it actually happened.

My two cents,
Jonathan