From: Junio C Hamano <hidden> Date: 2016-06-15 22:51:18
Jeff King [off-list ref] writes:
quoted
However, custom diff drivers (still) don't work. :-)
Yeah, I didn't add any support for that. I'm not sure what it should do;
custom diff drivers don't know how to handle combined diff, do they?
If you write me a test case that explains what _should_ happen, I'll see
what I can do. :)
I do not think it is sensible to expect anybody to come up with a sane
semantics for combined diff to work with GIT_EXTERNAL_DIFF (and external
diff driver that can be specified via the attributes mechanism) in any
meaningful way.
The whole point of the external diff mechanism is that an external command
can take _two_ files and represent the change between them in a way that
is more suited for the need of the user than the patch form. The output
from such an external command does not have any obligation to even follow
the convention used by the patch output, namely:
@@ from here to there things have changed @@
this is common
-this was the removed content
+this is the new content
as the _whole_ point of the external diff mechanism is to give something
that is _different_ from the patch form, in the hope that it is in a more
appropriate form for whoever consumes the output.
On the other hand, combined diff is all about combining multiple patches
show them side-by-side in a combined fashion. Without the above four kinds
of cues, there is no way to even _align_ the change outputs from two
parents, let alone _combining_ them.
Anybody interested can check "compare-cooking.perl" in the todo branch,
which is used as an external diff driver to view the differences between
"What's cooking" postings via these:
[diff "whatscooking"]
xfuncname = "^\\[(.*)\\]$"
command = ./compare-cooking.perl
in the .git/config file, together with
whats-cooking.txt diff=whatscooking
in the .gitattributes file. Running
$ git log -p --ext-diff todo -- whats-cooking.txt
would give a sample output.
It is conceivable that we _could_ newly define a "combined external diff
driver" that would take 3 or more files, and compute and show the combined
result by itself, but that will certainly not go through the codepath you
touched with the textconv patch. Calling out to such a new type of
external diff driver would have to happen at the level where we have 1+N
blob object names for a N-parent commit, namely, at the beginning of
show_patch_diff(), bypassing the entire contents of that function and
instead letting the new n-way external diff driver do everything.
I however highly doubt that such an interface would make sense. For
example, what would be the desirable format to compare three versions of
"What's cooking" postings, and how would the updated compare-cooking.perl
script would look like?
From: Michael J Gruber <hidden> Date: 2016-06-15 22:51:18
First of all:
Jeff, thanks a bunch for taking this up again! That's a great
improvement. (I'm not sure I can devote enough time to reviewing, but
I'll see.)
Junio C Hamano venit, vidit, dixit 24.05.2011 06:46:
Jeff King [off-list ref] writes:
quoted
quoted
However, custom diff drivers (still) don't work. :-)
Yeah, I didn't add any support for that. I'm not sure what it should do;
custom diff drivers don't know how to handle combined diff, do they?
If you write me a test case that explains what _should_ happen, I'll see
what I can do. :)
I do not think it is sensible to expect anybody to come up with a sane
semantics for combined diff to work with GIT_EXTERNAL_DIFF (and external
diff driver that can be specified via the attributes mechanism) in any
meaningful way.
The whole point of the external diff mechanism is that an external command
can take _two_ files and represent the change between them in a way that
is more suited for the need of the user than the patch form. The output
from such an external command does not have any obligation to even follow
the convention used by the patch output, namely:
@@ from here to there things have changed @@
this is common
-this was the removed content
+this is the new content
as the _whole_ point of the external diff mechanism is to give something
that is _different_ from the patch form, in the hope that it is in a more
appropriate form for whoever consumes the output.
On the other hand, combined diff is all about combining multiple patches
show them side-by-side in a combined fashion. Without the above four kinds
of cues, there is no way to even _align_ the change outputs from two
parents, let alone _combining_ them.
Anybody interested can check "compare-cooking.perl" in the todo branch,
which is used as an external diff driver to view the differences between
"What's cooking" postings via these:
[diff "whatscooking"]
xfuncname = "^\\[(.*)\\]$"
command = ./compare-cooking.perl
in the .git/config file, together with
whats-cooking.txt diff=whatscooking
in the .gitattributes file. Running
$ git log -p --ext-diff todo -- whats-cooking.txt
would give a sample output.
It is conceivable that we _could_ newly define a "combined external diff
driver" that would take 3 or more files, and compute and show the combined
result by itself, but that will certainly not go through the codepath you
touched with the textconv patch. Calling out to such a new type of
external diff driver would have to happen at the level where we have 1+N
blob object names for a N-parent commit, namely, at the beginning of
show_patch_diff(), bypassing the entire contents of that function and
instead letting the new n-way external diff driver do everything.
I however highly doubt that such an interface would make sense. For
example, what would be the desirable format to compare three versions of
"What's cooking" postings, and how would the updated compare-cooking.perl
script would look like?
Yeah, currently --cc with external makes no sense, but there are several
external tools which could present a 3-way diff in a useful way (or even
n-way with n>3), e.g. vimdiff, kdiff3, meld.
When the --cc/textconv issue came up I looked into this, and maybe
difftool is a place where one could plug this in first in the sense of
refactoring that even more and providing a diff3tool or such to view a
merge commit (or compare any 3 versions), or/and provide "git diff3 A B
C" which creates a fake merge (A+B -> C). Now, imagine we also have
INDEX and WORKTREE pseudorevs and can do
git diff3[tool] HEAD INDEX WORKTREE
:)
Michael
From: Jay Soffian <hidden> Date: 2016-06-15 22:51:18
On Tue, May 24, 2011 at 12:46 AM, Junio C Hamano [off-list ref] wrote:
Jeff King [off-list ref] writes:
quoted
quoted
However, custom diff drivers (still) don't work. :-)
Yeah, I didn't add any support for that. I'm not sure what it should do;
custom diff drivers don't know how to handle combined diff, do they?
If you write me a test case that explains what _should_ happen, I'll see
what I can do. :)
I do not think it is sensible to expect anybody to come up with a sane
semantics for combined diff to work with GIT_EXTERNAL_DIFF (and external
diff driver that can be specified via the attributes mechanism) in any
meaningful way.
Indeed, it was a thinko that I even considered it. It turns out I had
been using an external diff command where I should have been using
textconv all along.
j.
From: Jeff King <hidden> Date: 2016-06-15 22:51:18
On Tue, May 24, 2011 at 09:19:43AM +0200, Michael J Gruber wrote:
quoted
It is conceivable that we _could_ newly define a "combined external diff
driver" that would take 3 or more files, and compute and show the combined
result by itself, but that will certainly not go through the codepath you
touched with the textconv patch. Calling out to such a new type of
external diff driver would have to happen at the level where we have 1+N
blob object names for a N-parent commit, namely, at the beginning of
show_patch_diff(), bypassing the entire contents of that function and
instead letting the new n-way external diff driver do everything.
I however highly doubt that such an interface would make sense. For
example, what would be the desirable format to compare three versions of
"What's cooking" postings, and how would the updated compare-cooking.perl
script would look like?
Yeah, currently --cc with external makes no sense, but there are several
external tools which could present a 3-way diff in a useful way (or even
n-way with n>3), e.g. vimdiff, kdiff3, meld.
I agree with Junio that we would need a new config option and external
interface for "n-way combined diff". However, isn't what things like
vimdiff and meld do the reverse of our combined diff? That is, don't
they assume the 3 trees are: ours, theirs, and ancestor (i.e., merge
base)? Whereas in a combined diff, it is actually: merge parent 1
(ours), merge parent 2 (theirs), and merge _result_.
Also, do those tools generally handle n-way comparisons as opposed to
3-way?
-Peff
From: Michael J Gruber <hidden> Date: 2016-06-15 22:51:19
Jeff King venit, vidit, dixit 24.05.2011 21:13:
On Tue, May 24, 2011 at 09:19:43AM +0200, Michael J Gruber wrote:
quoted
quoted
It is conceivable that we _could_ newly define a "combined external diff
driver" that would take 3 or more files, and compute and show the combined
result by itself, but that will certainly not go through the codepath you
touched with the textconv patch. Calling out to such a new type of
external diff driver would have to happen at the level where we have 1+N
blob object names for a N-parent commit, namely, at the beginning of
show_patch_diff(), bypassing the entire contents of that function and
instead letting the new n-way external diff driver do everything.
I however highly doubt that such an interface would make sense. For
example, what would be the desirable format to compare three versions of
"What's cooking" postings, and how would the updated compare-cooking.perl
script would look like?
Yeah, currently --cc with external makes no sense, but there are several
external tools which could present a 3-way diff in a useful way (or even
n-way with n>3), e.g. vimdiff, kdiff3, meld.
I agree with Junio that we would need a new config option and external
interface for "n-way combined diff". However, isn't what things like
vimdiff and meld do the reverse of our combined diff? That is, don't
they assume the 3 trees are: ours, theirs, and ancestor (i.e., merge
base)? Whereas in a combined diff, it is actually: merge parent 1
(ours), merge parent 2 (theirs), and merge _result_.
Also, do those tools generally handle n-way comparisons as opposed to
3-way?
"Those" tools do "that" which I mean :)
It depends on the tool, of course. vimdiff does not have any assumptions
that I know of, it marks those hunks which are not common to all
buffers, and marks them differently depending on what subset of buffers
shares them. That aspect is the same for kdiff3 and meld (n<=3 for meld)
in diff mode.
There are more differences in merge mode:
In vimdiff, you can "put" a hunk (i.e. apply the change) to another
buffer (or "obtain" one), i.e. you can easily move hunks between the
buffers (to do a merge). In kdiff3's auto merge mode there is the
assumption you mention (because it actively does some merging).
So, in vimdiff, you could in principle change and write any buffer but
our tools don't support it so far because it is not needed for a merge
which has one "target" only. In the ui of my dreams, I would have 3
buffers HEAD INDEX WORKTREE (H I W) and moving hunks between them would
do all of add -p, reset -p and checkout -p (the HEAD buf would be
read-only).
With "differently abled" tools it could still be useful to have, say a
tool invoked for the merge HEAD+WORKTREE -> INDEX (with the current
state of the INDEX as the automatic resolution to start off from) so
that you can do add+reset -p with your mergetool, and maybe similarly
for HEAD+INDEX -> WORKTREE. I.e. addtool and checkouttool in addition to
the difftool and mergetool which we have. Just not for the current
release cycle any more.
Michael
From: Jeff King <hidden> Date: 2016-06-15 22:51:19
On Wed, May 25, 2011 at 09:38:43AM +0200, Michael J Gruber wrote:
quoted
I agree with Junio that we would need a new config option and external
interface for "n-way combined diff". However, isn't what things like
vimdiff and meld do the reverse of our combined diff? That is, don't
they assume the 3 trees are: ours, theirs, and ancestor (i.e., merge
base)? Whereas in a combined diff, it is actually: merge parent 1
(ours), merge parent 2 (theirs), and merge _result_.
Also, do those tools generally handle n-way comparisons as opposed to
3-way?
"Those" tools do "that" which I mean :)
OK, they are more capable than I realized, then. :)
So, in vimdiff, you could in principle change and write any buffer but
our tools don't support it so far because it is not needed for a merge
which has one "target" only. In the ui of my dreams, I would have 3
buffers HEAD INDEX WORKTREE (H I W) and moving hunks between them would
do all of add -p, reset -p and checkout -p (the HEAD buf would be
read-only).
This should be relatively easy to implement on the git side, as you are
doing the hard parts of the "-p" operation in vim already. You just need
to write vim's buffer into the worktree or into the index via
hash-object / update-index.
Also, wouldn't you potentially want more than 3 buffers, if you were
cherry-picking changes from another commit (as in "git checkout -p
$commit" or even putting and taking things from a stash? I think that
would be a simple generalization of what you are proposing though.
With "differently abled" tools it could still be useful to have, say a
tool invoked for the merge HEAD+WORKTREE -> INDEX (with the current
state of the INDEX as the automatic resolution to start off from) so
that you can do add+reset -p with your mergetool, and maybe similarly
for HEAD+INDEX -> WORKTREE. I.e. addtool and checkouttool in addition to
the difftool and mergetool which we have. Just not for the current
release cycle any more.
Yeah, getting back to the original discussion. How badly do people
actually want an external diff driver that can do fancy things with
multiple parents? It seems that for non-text diff viewers, the preferred
solution is to use difftool these days (but that is just my impression;
I don't use either difftool or external diff drivers).
-Peff