git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Оля Тележная" <olyatelezhnaya@gmail.com>
Cc: git <git@vger.kernel.org>
Subject: Re: [PATCH v2 03/18] ref-filter: make valid_atom as function parameter
Date: Wed, 17 Jan 2018 16:43:54 -0500	[thread overview]
Message-ID: <20180117214354.GA13128@sigill.intra.peff.net> (raw)
In-Reply-To: <CAL21Bm=+uPrKECcCq2_rfJRuCpsOjZ41NfiyY3d1UA0b8YKj1w@mail.gmail.com>

On Tue, Jan 16, 2018 at 09:55:22AM +0300, Оля Тележная wrote:

> > IOW, the progression I'd expect in a series like this is:
> >
> >   1. Teach ref-filter.c to support everything that cat-file can do.
> >
> >   2. Convert cat-file to use ref-filter.c.
> 
> I agree, I even made this and it's working fine:
> https://github.com/git/git/pull/450/commits/1b74f1047f07434dccb207534d1ad45a143e3f2b
> But I decided not to add that to patch because I expand the
> functionality of several commands (not only cat-file and
> for-each-ref), and I need to support all new functionality in a proper
> way, make these error messages, test everything and - the hardest one
> - support many new commands for cat-file. As I understand, it is not
> possible unless we finally move to ref-filter and print results also
> there. Oh, and I also need to rewrite docs in that case. And I decided
> to apply this in another patch. But, please, say your opinion, maybe
> we could do that here in some way.

Yeah, I agree that it will cause changes to other users of ref-filter,
and you'd need to deal with documentation and tests there. But I think
we're going to have to do those things eventually (since supporting
those extra fields everywhere is part of the point of the project). And
by doing them now, I think it can make the transition for cat-file a lot
simpler, because we never have to puzzle over this intermediate state
where only some of the atoms are valid for cat-file.

I also agree that moving cat-file's printing over to ref-filter.c is
going to introduce a lot of corner cases (like the behavior when
somebody does "cat-file --batch-check=%(refname)" with a bare sha1). But
I think a progression of patches handling those cases would be pretty
easy to follow. E.g., I'd expect to see a patch that teaches
ref-filter[1] how to handle callers that don't have a ref, but just a
bare object name. And it would be easier to evaluate that on its own,
because we know that's an end state we're going to have to handle, and
not some funny state created as part of the transition.

-Peff

[1] And here's where we might start to think about calling it something
    besides ref-filter, if we don't necessarily have a ref. :)

  reply	other threads:[~2018-01-17 21:43 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-09 13:05 [PATCH 01/20] cat-file: split expand_atom into 2 functions Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 14/20] cat-file: make populate_value global Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 13/20] cat-file: start use ref_array_item struct Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 04/20] cat-file: make valid_atoms as a function parameter Olga Telezhnaya
2018-01-09 22:16   ` Junio C Hamano
2018-01-09 13:05 ` [PATCH 05/20] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 02/20] cat-file: reuse struct ref_format Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 11/20] cat-file: get rid of duplicate checking Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 19/20] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 12/20] cat-file: rename variable in ref-filter Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 18/20] cat-file: add split_on_whitespace flag Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 20/20] cat-file: make cat_file_info variable independent Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 09/20] cat-file: get rid of goto in ref-filter Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 15/20] cat-file: start reusing populate_value Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 03/20] cat-file: rename variables in ref-filter Olga Telezhnaya
2018-01-09 22:04   ` Junio C Hamano
2018-01-10  7:07     ` Оля Тележная
2018-01-09 13:05 ` [PATCH 16/20] cat-file: get rid of expand_atom_into_fields Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 10/20] cat-file: simplify expand_atom function Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 17/20] cat-file: add is_cat flag in ref-filter Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 08/20] cat-file: remove unused code Olga Telezhnaya
2018-01-09 13:05 ` [PATCH 07/20] cat-file: reuse parse_ref_filter_atom Olga Telezhnaya
2018-01-09 23:32   ` Junio C Hamano
2018-01-09 13:05 ` [PATCH 06/20] cat-file: start migrating to ref-filter Olga Telezhnaya
2018-01-10  9:36 ` [PATCH v2 01/18] cat-file: split expand_atom into 2 functions Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 13/18] cat-file: start reusing populate_value Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 03/18] ref-filter: make valid_atom as function parameter Olga Telezhnaya
2018-01-15 21:42     ` Jeff King
2018-01-16  6:55       ` Оля Тележная
2018-01-17 21:43         ` Jeff King [this message]
2018-01-17 22:39           ` Christian Couder
2018-01-18  6:20             ` Оля Тележная
2018-01-18 11:49               ` Оля Тележная
2018-01-18 14:23                 ` Christian Couder
2018-01-19 12:24                   ` Оля Тележная
2018-01-19 15:48                     ` Christian Couder
2018-01-19 17:14               ` Christian Couder
2018-01-19 17:22                 ` Оля Тележная
2018-01-19 17:57                   ` Christian Couder
2018-01-19 17:23                 ` Jeff King
2018-01-19 17:31                   ` Christian Couder
2018-01-19 18:47                   ` Junio C Hamano
2018-01-19 20:12                     ` Jeff King
2018-01-10  9:36   ` [PATCH v2 06/18] ref-filter: reuse parse_ref_filter_atom Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
2018-01-15 21:44     ` Jeff King
2018-01-16  7:00       ` Оля Тележная
2018-01-17 21:45         ` Jeff King
2018-01-18  5:56           ` Оля Тележная
2018-01-10  9:36   ` [PATCH v2 02/18] cat-file: reuse struct ref_format Olga Telezhnaya
2018-01-15 21:37     ` Jeff King
2018-01-16  9:45       ` Оля Тележная
2018-01-10  9:36   ` [PATCH v2 14/18] ref-filter: get rid of expand_atom_into_fields Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 09/18] cat-file: simplify expand_atom function Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 10/18] cat-file: get rid of duplicate checking Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 07/18] cat-file: remove unused code Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 17/18] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 08/18] ref-filter: get rid of goto Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 16/18] ref_format: add split_on_whitespace flag Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 11/18] cat-file: start use ref_array_item struct Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 05/18] cat-file: start migrating to ref-filter Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 15/18] ref-filter: add is_cat flag Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 18/18] ref-filter: make cat_file_info independent Olga Telezhnaya
2018-01-10  9:36   ` [PATCH v2 12/18] ref-filter: make populate_value global Olga Telezhnaya
2018-01-15 21:33   ` [PATCH v2 01/18] cat-file: split expand_atom into 2 functions Jeff King
2018-01-15 22:09     ` Jeff King
2018-01-16  7:22       ` Оля Тележная
2018-01-17 21:49         ` Jeff King
2018-01-17 23:04           ` Christian Couder
2018-01-18  6:22             ` Оля Тележная
2018-01-18  8:45               ` Christian Couder

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180117214354.GA13128@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).