Re: [PATCH 2/3] diff: allow turning on textconv explicitly for plumbing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 07, 2008 at 07:55:12PM -0800, Junio C Hamano wrote:

> But I am not 100% sure if the scripting part is "the right way".
> 
> If a script wants to take whatever Porcelain users are happy as the
> "presentation for human consumption" and pass that through as its own
> output to the end user, maybe it is better off reading from Porcelain,
> instead of reading from the plumbing (the latter of which requires making
> the plumbing output less reliable)?

But I don't think having gitk call the porcelain makes sense, either.
That would enable, for example, external diff drivers which might
actually spawn an entirely new window. And I don't think most people
would want that from gitk.

Now, obviously it could call "git diff --no-ext-diff" to opt out of
that. But I think it is backwards to call porcelain but opt out of
features that you don't want. It makes more sense to start with the bare
minimum (i.e., plumbing) and opt in to features that you are OK with.

And yes, you increase the risk that a script which blindly passes
options to plumbing may now be asked to generate a textconv'd diff which
will be useless to the user. But:

  1. that is already a risk with --ext-diff

  2. it is actually a _feature_. It is asking the user to say "is this
     a sane option to pass for my situation or not" instead of forcing
     the script to make that decision.

> When we later enhance textconv output from the "diff" Porcelain to
> benefit interactive users, it will automatically help the script that
> passes through the "diff" output to the end users.

I'm not sure what enhancement you mean. But it is possible that gitk
would not _want_ such an enhancement, and it would then have to opt out
of it.

> You can certainly argue that this "textconv" feature that is grafted from
> Porcelain into plumbing is a special case in that its output is subject to
> change any time to help human consumption and we never strive for its
> stability as we do for other features in the plumbing to support machine
> readability by scripts.  You can propagate the later enhancement of
> textconv diff output we'd make for Porcelain to the scripted users that
> reads from the plumbing that way.

Right. _If_ it's a change that won't upset any plumbing consumers. If it
is, then it needs to be a separate option so that the plumbing consumers
who don't mind the change can start using it.

> But then wouldn't it be the same for these scripts that do value the
> "presentation meant for human consumption" over "machine readability" to
> read from Porcelain?  That would not have to blur the distinction between
> the Porcelain and plumbing like the approach you are suggesting here.

I don't agree that this is blurring the distinction. I don't see
textconv diffs as a "porcelain feature" that has been grafted on to
plumbing. I see it as a core diff feature which can be turned off or on.
Porcelain has it on by default, and plumbing has it off by default. But
you can ask either to change their default settings[1].

-Peff

[1]: You can argue that the current implementation is buggy in this
sense, though, since some porcelains will accept "--textconv" but not
change their behavior. I think blame, for example, will have that
problem, because it accepts diff options but does not necessarily use
them to create diffs. But I think that just means we have a bug and
eventually should handle the case of "git blame --textconv" correctly.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux