All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Оля Тележная" <olyatelezhnaya@gmail.com>
To: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, Christian Couder <christian.couder@gmail.com>
Subject: ref-filter: how to improve the code
Date: Sun, 25 Feb 2018 21:28:25 +0300	[thread overview]
Message-ID: <CAL21Bmnz=H96EE=yerdigujYQ6M7Y_U-RkDF7oR-UPDU+cZNFA@mail.gmail.com> (raw)

Hi everyone,
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.

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.
The draft of the code is here: https://github.com/telezhnaya/git/commits/p2

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.

If you have another ideas - please share them with me. It is really
important step to make formatting logic more general and easier to
reuse.

Thanks!
Olga

             reply	other threads:[~2018-02-25 18:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-25 18:28 Оля Тележная [this message]
2018-02-28 13:25 ` ref-filter: how to improve the code Jeff King
2018-03-01 11:17   ` Оля Тележная
2018-03-01 15:04     ` Jeff King

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='CAL21Bmnz=H96EE=yerdigujYQ6M7Y_U-RkDF7oR-UPDU+cZNFA@mail.gmail.com' \
    --to=olyatelezhnaya@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.