From: Jeff King <peff@peff.net>
To: "Оля Тележная" <olyatelezhnaya@gmail.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
Christian Couder <christian.couder@gmail.com>
Subject: Re: ref-filter: how to improve the code
Date: Thu, 1 Mar 2018 10:04:52 -0500 [thread overview]
Message-ID: <20180301150451.GC24907@sigill.intra.peff.net> (raw)
In-Reply-To: <CAL21BmkVq4j=hgupPvZqigSGQ-=rgwKVvzTD_X-_Z__8qmeKJg@mail.gmail.com>
On Thu, Mar 01, 2018 at 02:17:09PM +0300, Оля Тележная wrote:
> >> 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).
>
> Not sure if you catch my idea. format_ref_array_item() will not print
> anything, it will just return an error code. And if there was an error
> - we will print it in show_ref_array_item() (or go back to cat-file
> and print what we want).
OK, I think I misunderstood. It seems like there are three possible
strategies on the table:
- low-level functions call error() and return -1, that gets passed up
through mid-level functions like format_ref_array_item(), and then
higher-level functions like show_ref_array_item() act on the error
code and call die(). The user sees something like:
error: unable to parse object 1234abcd
fatal: unable to format object
- low-level functions return a numeric error code, which is then
formatted by higher-level functions like show_ref_array_item() to
produce a specific message
- low-level functions stuff an error code into a strbuf and return -1,
and then higher-level functions like show_ref_array_item() will feed
that message to die("%s", err.buf).
I think the first one, besides changing the output, is going to produce
error() messages even for cases where we're calling
format_ref_array_item() directly, because error() writes its output
immediately.
The second is a pain in practice, because it doubles the work: you have
to come up with a list of error codes, and then translate it them into
strings. And there's no room to mention variable strings (like the name
of the object).
So I think the third is really the only viable option.
-Peff
prev parent reply other threads:[~2018-03-01 15:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-25 18:28 ref-filter: how to improve the code Оля Тележная
2018-02-28 13:25 ` Jeff King
2018-03-01 11:17 ` Оля Тележная
2018-03-01 15:04 ` Jeff King [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180301150451.GC24907@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=olyatelezhnaya@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.