Re: ref-filter: how to improve the code

On Sun, Feb 25, 2018 at 09:28:25PM +0300, Оля Тележная wrote:

I am trying to remove cat-file formatting part and reuse same
functionality from ref-filter.
I have a problem that cat-file sometimes needs to continue running
even if the request is broken, while in ref-filter we invoke die() in
many places everywhere during formatting process. I write this email
because I want to discuss how to implement the solution better.

ref-filter has 2 functions which could be interesting for us:
format_ref_array_item() and show_ref_array_item(). I guess it's a good
idea to print everything in show_ref_array_item(), including all
errors. In that case all current users of ref-filter will invoke
show_ref_array_item() (as they did it before), and cat-file could use
format_ref_array_item() and work with the result in its own logic.

Yes, that arrangement makes sense to me.

I tried to replace all die("...") with `return error("...")` and
finally exit(), but actual problem is that we print "error:..."
instead of "fatal:...", and it looks funny.

If you do that, then format_ref_array_item() is still going to print
things, even if it doesn't die(). But for "cat-file --batch", we usually
do not print errors at all, but instead just say "... missing" (although
it depends on the error; syntactic errors in the format string would
still cause us to write to stderr).

One of the easiest solutions is to add strbuf parameter for errors to
all functions that we use during formatting process, fill it in and
print in show_ref_array_item() if necessary. What do you think about
this idea?

Another way is to change the resulting error message, print current
message with "error" prefix and then print something like "fatal:
could not format the output". It is easier but I am not sure that it's
a good idea to change the interface.

For reference, the first one is what we've been switching to in the refs
code. And I think it's been fairly successful there.

-Peff