From: Michael J Gruber <git@drmicha•warpmail.net>
To: Junio C Hamano <gitster@pobox•com>, John Keeping <john@keeping•me.uk>
Cc: Florian Aspart <florian.aspart@gmail•com>, git@vger•kernel.org
Subject: Re: Using clean/smudge filters with difftool
Date: Fri, 19 Jun 2015 10:57:55 +0200 [thread overview]
Message-ID: <5583D993.4090305@drmicha.warpmail.net> (raw)
In-Reply-To: <xmqqr3p8bp8l.fsf@gitster.dls.corp.google.com>
Junio C Hamano venit, vidit, dixit 19.06.2015 00:55:
> John Keeping <john@keeping•me.uk> writes:
>
>> I think the summary is that there are some scenarios where the external
>> diff tool should see the smudged version and others where the clean
>> version is more appropriate and Git should support both options. It
>> seems this is a property of the filter, so I wonder if the best solution
>> is a new "filter.<name>.extdiff = [clean|smudge]" configuration
>> variable (there's probably a better name for the variable than
>> "extdiff").
>
> Not just the external diff, but the textconv filter obeys the same
> rule. The setting should be done the same way for both, if we are
> going to go in that direction.
>
textconv is a "one-way" filter from "blob" to "readable blob". External
diffs may prefer to work on "blob" rather than "readable blob", but the
currect setup does not seem to produce surprises.
clean and smudge are two-way filters: clean from "worktree blob" (aka
file) to "repo blob", smudge the other way round.
Typically, the user perceives these as inverse to each other. But we
only require clean to be a left-inverse of smudge, i.e. "(cat-file then)
smudge then clean" should give the same "repo blob" (as "cat-file").
We don't require that the other way round, i.e. we don't require smudge
to be a left-inverse of clean, and in most setups (like the current one)
it is not: smudge does not recreate what clean has cleaned out. It is a
no-op (the "identity", while clean is a "projection").
Now, since external diff runs on smudged blobs, it appears as if we
mixed cleaned and smudged blobs when feeding external diffs; whereas
really, we mix "worktree blobs" and "smudged repo blobs", which is okay
as per our definition of clean/smudge: the difference is irrelevant by
definition.
I still think that feeding cleaned blobs to external diff would be less
surprising (and should be the default, but maybe can't be changed any
more) and feeding smudged blobs should be the special case requiring a
special config. Because otherwise, the external diff would have to know
which parts of the diff are irrelevant - if it display the complete
("uncleaned") diff, it shows differences ("what will be committed") that
will not end up in the commit (because they will get cleaned out before).
As a guiding principle, a worktree-HEAD diff and an index-HEAD diff
should be previews of the result of "commit -a" resp. "commit", and
therefore should diff cleaned versions. textconv, on the other hand, is
a setting by which you tell git: "Don't show me the 'proper'
diff/commit-preview but a readable version."
Michael
next prev parent reply other threads:[~2015-06-19 8:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 14:11 Using clean/smudge filters with difftool Florian Aspart
2015-06-18 12:31 ` Michael J Gruber
2015-06-18 13:15 ` Florian Aspart
2015-06-18 13:26 ` John Keeping
2015-06-18 13:51 ` Florian Aspart
2015-06-18 14:11 ` John Keeping
2015-06-18 14:17 ` Florian Aspart
2015-06-18 14:28 ` John Keeping
2015-06-18 15:39 ` Florian Aspart
2015-06-18 16:01 ` John Keeping
2015-06-18 20:00 ` Junio C Hamano
2015-06-18 22:39 ` John Keeping
2015-06-18 22:55 ` Junio C Hamano
2015-06-19 8:57 ` Michael J Gruber [this message]
2015-06-19 9:32 ` John Keeping
2015-06-19 15:04 ` Florian Aspart
2015-06-19 17:03 ` Junio C Hamano
2015-06-21 19:29 ` Michael J Gruber
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=5583D993.4090305@drmicha.warpmail.net \
--to=git@drmicha$(echo .)warpmail.net \
--cc=florian.aspart@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=john@keeping$(echo .)me.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox