* Using Filemerge.app as a git-diff viewer @ 2007-11-21 10:31 Toby White 2007-11-21 11:20 ` Jonathan del Strother 2007-11-21 11:27 ` Jeff King 0 siblings, 2 replies; 9+ messages in thread From: Toby White @ 2007-11-21 10:31 UTC (permalink / raw) To: git; +Cc: Wincent Colaiuta Mac OS X bundles a rather nice graphical diff viewer (Filemerge.app) with its developer tools. While git-merge knows how to use this as a merge tool, I couldn't find any way to easily use Filemerge as a viewer for the output of git-diff. (http://thread.gmane.org/gmane.comp.version-control.git/58702 discusses the problem, and recommends piping git-diff into kompare. Filemerge unfortunately won't accept diff output on stdin) So I wrote a quick script (below) which does what I need. Of all the available git-diff flags, it only understands "--cached", and up to two commit objects, and no paths, but that's enough for me. Within those constraints, it has the same semantics as git-diff. It's not very nice, but in case anyone else wants this: #!/bin/sh # # Filemerge.app must not already be open before running # this script, or opendiff below will return immediately, # and the TMPDIRs deleted before it gets the chance to read # them. if test $# = 0; then OLD=`git-write-tree` elif test "$1" = --cached; then OLD=HEAD NEW=`git-write-tree` shift fi if test $# -gt 0; then OLD="$1"; shift fi test $# -gt 0 && test -z "$CACHED" && NEW="$1" TMPDIR1=`mktemp -d` git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -) if test -z "$NEW"; then TMPDIR2=$(git rev-parse --show-cdup) test -z "$cdup" && TMPDIR2=. else TMPDIR2=`mktemp -d` git-archive --format=tar $NEW | (cd $TMPDIR2; tar xf -) fi opendiff $TMPDIR1 $TMPDIR2 | cat rm -rf $TMPDIR1 test ! -z "$NEW" && rm -rf $TMPDIR2 -- Dr. Toby O. H. White Dept. Earth Sciences, Downing Street, Cambridge CB2 3EQ United Kingdom Tel: +44 1223 333464 Fax: +44 1223 333450 Web: http://uszla.me.uk ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White @ 2007-11-21 11:20 ` Jonathan del Strother 2007-11-21 11:27 ` Jeff King 1 sibling, 0 replies; 9+ messages in thread From: Jonathan del Strother @ 2007-11-21 11:20 UTC (permalink / raw) To: Toby White; +Cc: git, Wincent Colaiuta On 21 Nov 2007, at 10:31, Toby White wrote: > Mac OS X bundles a rather nice graphical diff viewer (Filemerge.app) > with its developer tools. > > While git-merge knows how to use this as a merge tool, I couldn't > find any way to easily use Filemerge as a viewer for the output > of git-diff. > > (http://thread.gmane.org/gmane.comp.version-control.git/58702 > discusses the problem, and recommends piping git-diff into kompare. > Filemerge unfortunately won't accept diff output on stdin) > > So I wrote a quick script (below) which does what I need. Of all > the available git-diff flags, it only understands "--cached", and > up to two commit objects, and no paths, but that's enough for me. > Within those constraints, it has the same semantics as git-diff. > ... Handy, thanks. Just as a note to anyone who runs into the same problems as me - I couldn't persuade this script to work at first. On my Leopard install, mktemp needs a template argument to run. After replacing both `mktemp -d`s in this script with `mktemp -d /tmp/diff.XXXXX`, it works nicely. Jon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White 2007-11-21 11:20 ` Jonathan del Strother @ 2007-11-21 11:27 ` Jeff King 2007-11-21 12:59 ` Wincent Colaiuta 2007-11-21 13:10 ` Toby White 1 sibling, 2 replies; 9+ messages in thread From: Jeff King @ 2007-11-21 11:27 UTC (permalink / raw) To: Toby White; +Cc: git, Wincent Colaiuta On Wed, Nov 21, 2007 at 10:31:46AM +0000, Toby White wrote: > So I wrote a quick script (below) which does what I need. Of all > the available git-diff flags, it only understands "--cached", and > up to two commit objects, and no paths, but that's enough for me. > Within those constraints, it has the same semantics as git-diff. Have you looked at the documentation for GIT_EXTERNAL_DIFF (try git(7))? I think it is a cleaner way of doing what you want (although I think you will get each file diffed individually, which is perhaps not what you want). Something like: $ cat >merge.sh <<EOF #!/bin/sh opendiff "$1" "$2" EOF $ GIT_EXTERNAL_DIFF=./merge.sh git-diff ... > #!/bin/sh > # > # Filemerge.app must not already be open before running > # this script, or opendiff below will return immediately, > # and the TMPDIRs deleted before it gets the chance to read > # them. > > if test $# = 0; then > OLD=`git-write-tree` > elif test "$1" = --cached; then > OLD=HEAD > NEW=`git-write-tree` > shift > fi > if test $# -gt 0; then > OLD="$1"; shift > fi > test $# -gt 0 && test -z "$CACHED" && NEW="$1" write-tree? Yikes. If you want to diff against the working tree, then do that. If you want to diff against the index, then you probably want to git-checkout-index to a tmpdir, and diff against that. > git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -) Again, this could be simpler and faster by using git-checkout-index (preceded by git-read-tree into a temp index, if you are comparing against a tree). -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 11:27 ` Jeff King @ 2007-11-21 12:59 ` Wincent Colaiuta 2007-11-21 13:04 ` Jeff King 2007-11-21 13:58 ` Jakub Narebski 2007-11-21 13:10 ` Toby White 1 sibling, 2 replies; 9+ messages in thread From: Wincent Colaiuta @ 2007-11-21 12:59 UTC (permalink / raw) To: Jeff King; +Cc: Toby White, git El 21/11/2007, a las 12:27, Jeff King escribió: > On Wed, Nov 21, 2007 at 10:31:46AM +0000, Toby White wrote: > >> So I wrote a quick script (below) which does what I need. Of all >> the available git-diff flags, it only understands "--cached", and >> up to two commit objects, and no paths, but that's enough for me. >> Within those constraints, it has the same semantics as git-diff. > > Have you looked at the documentation for GIT_EXTERNAL_DIFF (try > git(7))? > I think it is a cleaner way of doing what you want (although I think > you > will get each file diffed individually, which is perhaps not what you > want). > > Something like: > > $ cat >merge.sh <<EOF > #!/bin/sh > opendiff "$1" "$2" > EOF > $ GIT_EXTERNAL_DIFF=./merge.sh git-diff ... A few problems with that: - FileMerge is broken, and crashes if you pass /dev/null as a param (which happens for new/deleted files) - you need to escape those $-signs - the params you're interested in are actually $2 and $5, not $1 and $2, according to git(7) - you need to handle the clean tree case (no params are passed) - and also the 1-param case, "for unmerged paths", whatever that means - will only work when run from the top of the tree (path parameters are passed in relative to that) So here's a less-broken version of your suggestion, but it's still broken; a relatively complex wrapper is required to do this right: $ cat >merge.sh <<EOF #!/bin/sh [ \$# -eq 7 ] && opendiff "\$2" "\$5" EOF chmod +x merge.sh GIT_EXTERNAL_DIFF=./merge.sh git-diff ... Cheers, Wincent ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 12:59 ` Wincent Colaiuta @ 2007-11-21 13:04 ` Jeff King 2007-11-21 13:58 ` Jakub Narebski 1 sibling, 0 replies; 9+ messages in thread From: Jeff King @ 2007-11-21 13:04 UTC (permalink / raw) To: Wincent Colaiuta; +Cc: Toby White, git On Wed, Nov 21, 2007 at 01:59:37PM +0100, Wincent Colaiuta wrote: > So here's a less-broken version of your suggestion, but it's still broken; a > relatively complex wrapper is required to do this right: Thanks. My "something like" should more accurately have been "I really didn't think about this at all." But I do think a GIT_EXTERNAL_DIFF-type approach (and if GIT_EXTERNAL_DIFF isn't powerful enough, considering some alternate interface) is nicer than trying to wrap git-diff. > $ cat >merge.sh <<EOF > #!/bin/sh > [ \$# -eq 7 ] && opendiff "\$2" "\$5" > EOF This is probably easier to read using <<'EOF' to quote (but I failed to do even that in my example). -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 12:59 ` Wincent Colaiuta 2007-11-21 13:04 ` Jeff King @ 2007-11-21 13:58 ` Jakub Narebski 1 sibling, 0 replies; 9+ messages in thread From: Jakub Narebski @ 2007-11-21 13:58 UTC (permalink / raw) To: git Wincent Colaiuta wrote: > - you need to escape those $-signs [...] > $ cat >merge.sh <<EOF Or use $ cat >merge.sh <<\EOF -- Jakub Narebski Warsaw, Poland ShadeHawk on #git ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 11:27 ` Jeff King 2007-11-21 12:59 ` Wincent Colaiuta @ 2007-11-21 13:10 ` Toby White 2007-11-21 13:28 ` Wincent Colaiuta 2007-11-21 14:36 ` Jeff King 1 sibling, 2 replies; 9+ messages in thread From: Toby White @ 2007-11-21 13:10 UTC (permalink / raw) To: Jeff King; +Cc: git, Wincent Colaiuta Jeff King wrote: > Have you looked at the documentation for GIT_EXTERNAL_DIFF (try git(7))? > I think it is a cleaner way of doing what you want (although I think you > will get each file diffed individually, which is perhaps not what you > want). > > Something like: > > $ cat >merge.sh <<EOF > #!/bin/sh > opendiff "$1" "$2" > EOF > $ GIT_EXTERNAL_DIFF=./merge.sh git-diff ... I hadn't seen that, no, but that's not quite right. (Wincent pointed out its flaws better than me. Basically, opendiff is not really diff-like enough.) And in any case, that launches Filemerge repeatedly on every file separately, which makes reviewing a large diff time-consuming and not very helpful. > write-tree? Yikes. If you want to diff against the working tree, then do > that. If you want to diff against the index, then you probably want to > git-checkout-index to a tmpdir, and diff against that. Am I misunderstanding the documentation? From man git-write-tree "Conceptually, git-write-tree sync()s the current index contents into a set of tree files. In order to have that match what is actually in your directory right now, you need to have done a git-update-index phase before you did the git-write-tree." So git-write-tree precisely does give you the index not the working tree, by my reading. >> git-archive --format=tar $OLD | (cd $TMPDIR1; tar xf -) > > Again, this could be simpler and faster by using git-checkout-index > (preceded by git-read-tree into a temp index, if you are comparing > against a tree). Erm, ok, this is rapidly approaching the limit of my git knowledge, but while I can see git-read-tree will write a tree-ish into a temp index, (so presumably git-read-tree --index-output=$TMPFILE <commit> ought to work. Except it doesn't, I get the error message fatal: unable to write new index file ), I can't see how to make git-checkout-index read from a temp index. And I'm assuming I don't want to go stomping all over the current index just in order to do a diff, which shouldn't change the state of my repository. Is there a canonical way to checkout a given commit object into a fresh directory? -- Dr. Toby O. H. White Dept. Earth Sciences, Downing Street, Cambridge CB2 3EQ United Kingdom Tel: +44 1223 333464 Fax: +44 1223 333450 Web: http://uszla.me.uk ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 13:10 ` Toby White @ 2007-11-21 13:28 ` Wincent Colaiuta 2007-11-21 14:36 ` Jeff King 1 sibling, 0 replies; 9+ messages in thread From: Wincent Colaiuta @ 2007-11-21 13:28 UTC (permalink / raw) To: Toby White; +Cc: Jeff King, git El 21/11/2007, a las 14:10, Toby White escribió: > (Wincent pointed out its flaws better than me. Basically, > opendiff is not really diff-like enough.) > > And in any case, that launches Filemerge repeatedly > on every file separately, which makes reviewing a large diff > time-consuming and not very helpful. I know it's not much help to you right now, but when I first asked about a side-by-side diff viewer back in September I explored creating a wrapper using GIT_EXTERNAL_DIFF and was basically unsatisfied with the results. So I decided to write a very simple native Cocoa app (I'm working on Mac OS X); it was to be a diff viewer and nothing more (not a repository browser). I was able to spend a few hours on it back in September but then other things came up so I haven't been able to finish it, but I do intend to come back to it when time permits. You can browse the work in progress at: <http://git.wincent.com/gdiff.git> Basically consists of a fast Ragel-generated state machine for parsing git-diff output, patches and email messages, and the beginnings of a UI. But like I said, not much help to you right now as it can't yet be used to do anything useful. My goal for it is to provide a nice experience, above all when reviewing patches on a mailing list (ie. where your local repository doesn't have the proposed commits in it yet). In fact, even when run from outside of any Git repo I want it to at least show the known parts of the files (the context) and provide a shaded background for the unknown bits. Cheers, Wincent ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Using Filemerge.app as a git-diff viewer 2007-11-21 13:10 ` Toby White 2007-11-21 13:28 ` Wincent Colaiuta @ 2007-11-21 14:36 ` Jeff King 1 sibling, 0 replies; 9+ messages in thread From: Jeff King @ 2007-11-21 14:36 UTC (permalink / raw) To: Toby White; +Cc: git, Wincent Colaiuta On Wed, Nov 21, 2007 at 01:10:47PM +0000, Toby White wrote: > And in any case, that launches Filemerge repeatedly > on every file separately, which makes reviewing a large diff > time-consuming and not very helpful. I think that particular complaint of GIT_EXTERNAL_DIFF has come up before. It probably wouldn't be too hard to support a separate environment variable to mean "diff these two trees" (or tree/cache, or cache/working tree, etc) rather than "diff these two files". > Am I misunderstanding the documentation? From man git-write-tree > > "Conceptually, git-write-tree sync()s the current index contents into a > set of tree files. In order to have that match what is actually in your > directory right now, you need to have done a git-update-index phase > before you did the git-write-tree." > > So git-write-tree precisely does give you the index not the working tree, > by my reading. What you are doing isn't _wrong_, but it is not the most efficient way of doing it, and it is a little bit awkward, I think. There are basically three conceptual "spots" for files in git: - in the object database (i.e., tree objects pointing to blobs) - in the index (the blobs are actually in the object db) - in the working tree (i.e., actual files) git-write-tree moves data from the index into the object database (git-commit is more or less git-write-tree followed by git-commit-tree). git-archive (piped to tar) moves data from the object database (because it takes a tree object name) into the working tree. So you are basically moving data into the object db, and then out to the working tree. But there is a command that moves data directly from the index to the working tree: git-checkout-index. Now, what you are doing is not terribly inefficient in that sense, since creating a tree object is usually pretty inexpensive. But it does involve writing to the object db for a diff, which is conceptually a read-only operation. However, git-checkout-index also avoids piping through tar, which is likely to be faster for a large data set. > Erm, ok, this is rapidly approaching the limit of my git knowledge, BTW, I don't mean to nitpick your script, which obviously works for you. I'm just trying to increase your git knowledge. :) > but while I can see git-read-tree will write a tree-ish into a temp > index, > > (so presumably > git-read-tree --index-output=$TMPFILE <commit> > ought to work. Except it doesn't, I get the error message > fatal: unable to write new index file > ), Are you perhaps running against the rename(2)-ability boundaries mentioned in the man page? --index-output is relatively new, and I'm not sure of the reasons surrounding it. I think what you really want to work with a temp index is to set GIT_INDEX_FILE. > I can't see how to make git-checkout-index read from a temp index. It should read from GIT_INDEX_FILE. So the correct incantation for checking out a tree should be: GIT_INDEX_FILE=$TMPFILE git-read-tree <commit> GIT_INDEX_FILE=$TMPFILE git-checkout-index -a --prefix=$TMPDIR/ and of course, for the --cached case, you can just use the existing index: git-checkout-index -a --prefix=$TMPDIR/ (btw, I have no idea if creating a temp index is actually faster than piping through tar. I suspect it is, but haven't tested. So it may be sane to use checkout-index for the --cached case, which is what I was originally suggesting, and simply use git-archive for the tree case). > And I'm assuming I don't want to go stomping all over the current > index just in order to do a diff, which shouldn't change the state > of my repository. Correct. > Is there a canonical way to checkout a given commit object into > a fresh directory? git-archive piped to tar is the canonical porcelain way (again, I was suggesting checkout-index mainly for the --cached case). The way I showed above is the "plumbing" way (and is what underlies git-checkout, for example, though of course a temp index is not needed in that case). Hope that make sense. -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-11-21 14:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-21 10:31 Using Filemerge.app as a git-diff viewer Toby White 2007-11-21 11:20 ` Jonathan del Strother 2007-11-21 11:27 ` Jeff King 2007-11-21 12:59 ` Wincent Colaiuta 2007-11-21 13:04 ` Jeff King 2007-11-21 13:58 ` Jakub Narebski 2007-11-21 13:10 ` Toby White 2007-11-21 13:28 ` Wincent Colaiuta 2007-11-21 14:36 ` Jeff King
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 NNTP newsgroup(s).