git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Stefan Beller <sbeller@google.com>,
	Jeff Smith <whydoubt@gmail.com>,
	"git\@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [RFC PATCH 02/10] Move textconv_object to be with other textconv methods
Date: Tue, 09 May 2017 10:49:19 +0900	[thread overview]
Message-ID: <xmqqziem6bq8.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170508215525.gcyzzntqvm52mqcc@sigill.intra.peff.net> (Jeff King's message of "Mon, 8 May 2017 17:55:25 -0400")

Jeff King <peff@peff.net> writes:

> The specific helpers have less visibility, which is good. The public
> functions a() and b() were already public, so no change. But now the
> common helper is public, too, even though nobody except a() and b() care
> about it.
>
> So it's a tradeoff. And the important question is: is the bleed-over
> between a() and b() worse than the common bits being made public?

At this point you are saying the same thing as I said ;-) 

I haven't touched diff.c for a while, but my impression was that it
was already a smallest logical unit, especially since some bits like
diff-lib.c (the interface for the front-end programs to drive the
machinery via the three standard pairs of sources) and ws.c (diff
subsystem to deal with whitespace errors) are already split out (and
needless to say, the diffcore transformations are all in their own
separate files).  Over time people may have added what does not
belong there merely for convenience, which may want to get ejected,
but I do not think of many instances of them offhand.

    ... goes and looks ...

It appears that the textconv related helpers (which was where this
discussion thread started from) could be first restructured so that
they do not depend on diff_filespec and turned into a more generic
"I have this path and a blob object name, please run textconv to it
and..." interface and then moved out of the file into its own
textconv.[ch].  It does not need access to quite a few fields in
diff_filespec structure (e.g.  it shouldn't care what filemode the
thing has).  The diff machinery wants the result in a contiguous
piece of memory and that could be a good starting point, but I said
"and..." above because I am not sure if it is the best generic
thing the restructured interface can do to the result.






  parent reply	other threads:[~2017-05-09  1:49 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-05  5:27 [RFC PATCH 00/10] Add blame to libgit Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 01/10] Remove unneeded dependency on blob.h from blame Jeff Smith
2017-05-05  7:07   ` Ævar Arnfjörð Bjarmason
     [not found]     ` <CAPX7N=6tQi+WidagvV1BA-CoaiNJj7OO4U7GYXNE-QzyvD=QRQ@mail.gmail.com>
2017-05-05 14:03       ` Jeffrey Smith
2017-05-05  5:27 ` [RFC PATCH 02/10] Move textconv_object to be with other textconv methods Jeff Smith
2017-05-05 17:44   ` Junio C Hamano
2017-05-05 17:50     ` Stefan Beller
2017-05-08  1:02       ` Junio C Hamano
2017-05-08 21:55         ` Jeff King
2017-05-08 22:06           ` Stefan Beller
2017-05-09  1:49           ` Junio C Hamano [this message]
2017-05-09  2:34             ` Jeff King
2017-05-05  5:27 ` [RFC PATCH 03/10] Add some missing definitions to header files Jeff Smith
2017-05-05 17:42   ` Junio C Hamano
2017-05-05  5:27 ` [RFC PATCH 04/10] Remove unused parameter from get_origin() Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 05/10] Split blame origin into its own file Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 06/10] Move fake_working_tree_commit() to lib Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 07/10] Break out scoreboard a little better Jeff Smith
2017-05-05 17:52   ` Junio C Hamano
2017-05-05  5:27 ` [RFC PATCH 08/10] Split blame scoreboard into its own file Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 09/10] Break out scoreboard init and setup Jeff Smith
2017-05-05  5:27 ` [RFC PATCH 10/10] Move scoreboard init and setup to lib Jeff Smith
2017-05-05 17:54 ` [RFC PATCH 00/10] Add blame to libgit Junio C Hamano
2017-05-14  3:14 ` [RFC PATCH v2 00/22] " Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 01/22] blame: remove unneeded dependency on blob.h Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 02/22] blame: move textconv_object with related functions Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 03/22] blame: remove unused parameters Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 04/22] blame: move origin and entry structures to header Jeff Smith
2017-05-14  8:10     ` Junio C Hamano
2017-05-14  3:14   ` [RFC PATCH v2 05/22] blame: move scoreboard structure " Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 06/22] blame: move stat counters to scoreboard Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 07/22] blame: move copy/move thresholds " Jeff Smith
2017-05-14  3:14   ` [RFC PATCH v2 08/22] blame: move contents_from " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 09/22] blame: move reverse flag " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 10/22] blame: move show_root " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 11/22] blame: move xdl_opts flags " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 12/22] blame: move no_whole_file_rename flag " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 13/22] blame: make sanity_check use a callback in scoreboard Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 14/22] blame: move progess updates to a scoreboard callback Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 15/22] blame: wrap blame_sort and compare_blame_final Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 16/22] blame: rework methods that determine 'final' commit Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 17/22] blame: move origin-related methods to libgit Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 18/22] blame: move fake-commit-related " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 19/22] blame: move scoreboard-related " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 20/22] blame: create scoreboard init function in libgit Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 21/22] blame: create scoreboard setup " Jeff Smith
2017-05-14  3:15   ` [RFC PATCH v2 22/22] blame: create entry prepend " Jeff Smith
2017-05-15  9:24   ` [RFC PATCH v2 00/22] Add blame to libgit Junio C Hamano
2017-05-15 13:52     ` Jeffrey Smith
2017-05-16  0:23       ` Junio C Hamano
2017-05-16  2:44         ` Jeffrey Smith
2017-05-16  3:21           ` Junio C Hamano

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=xmqqziem6bq8.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    --cc=whydoubt@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).