Re: [PATCH] stash: show error message when lockfile is present

Ævar Arnfjörð Bjarmason [off-list ref] writes:

The "quiet" here is orthagonal, it's to disable the chatty output from
read-cache.c.

It shouldn't be.  read-cache.c may want to report errors and
progresses, and among the errors it may not include lock-related
things.  If the change were to (1) extend the flag word to make it
more uniform, so that it does not special case ONLY the lock-related
errors, and to (2) figure out how to grab richer diagnoses out of
the lower level API functions, and to (3) reduce the messages issued
by the lower-level API functions and have it callers' responsibility
to give the messages, that would have been palatable, but the patch
as posted goes totally the opposite direction on all these points.

Yuck.

I would think the "fix" should look more like this:

 	read_cache_preload(NULL);
	if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
- 		return -1;
+ 		return error(_("failed to refresh the index"));

This is how the other callsites of refresh_and_write_cache (the ones
outside of stash.c) solve this problem, however I agree with Ævar that
this error is not as useful as the one given by
LOCK_REPORT_ON_ERROR (which is why I wrote the patch this way
to begin with, although I do see that it is not exactly the most elegant
solution to the problem)

If the change were to (1) extend the flag word to make it more uniform,
so that it does not special case ONLY the lock-related errors

Would adding a REFRESH_REPORT_ERRORS flag which could then be
used with repo_refresh_and_write_index be a more acceptable solution? 
Then various functions in read-cache.c could check for this flag and
output errors but not progress when it is present.

Regards,
Birk