git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Wincent Colaiuta <win@wincent.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: What's cooking in git.git (Mar 2009, #06; Sat, 21)
Date: Sat, 21 Mar 2009 11:58:18 -0700	[thread overview]
Message-ID: <20090321185817.GA22540@gmail.com> (raw)
In-Reply-To: <0768D909-FCD5-4E5B-95A7-2602824FC431@wincent.com>

On  0, Wincent Colaiuta <win@wincent.com> wrote:
> El 21/3/2009, a las 8:58, Junio C Hamano escribió:
>
>> * da/difftool (Thu Mar 19 01:25:25 2009 -0700) 1 commit
>> - difftool: move 'git-difftool' out of contrib
>
> Before this one goes any further, I noticed that nobody replied to my  
> email on this thread a few days ago:
>
> http://article.gmane.org/gmane.comp.version-control.git/113609
>
> My concern was:
>
>> Given that git-difftool shares basically all the same options as "git 
>> diff", I think a good long-term plan would be looking at adding the 
>> "--tool" option to "git diff" itself so that users wouldn't have to 
>> learn a new subcommand, just a new option.
>
>
> What do people think?
>
> Cheers,
> Wincent

That could be interesting.  git-difftool is just a
frontend to git-diff so there isn't really any maintenance
worries about keeping the options in sync.  I do agree that
keeping things easy for users is a noble goal and that that
was your only concern.

git-difftool is pure porcelain, so I'm interested in how we
could implement this.  Right now the call stack is:

$ git difftool
... GIT_EXT_DIFF=git-difftool-helper
... git diff
... ... git-difftool-helper
... ... ... xxdiff


What should it look like instead?

Are you envisioning this (1):

$ git diff --tool
... --tool was passed, so set GIT_EXT_DIFF?
... git-difftool-helper
... ... xxdiff ...


Or do you mean this? (2):

$ git diff --tool=xxdiff
... --tool was passed, so set GIT_EXT_DIFF?
... git-difftool-helper
... ... xxdiff


Or even... (3):

$ git diff --tool
... --tool was passed, delegate to git-diff--tool,
...        remove --tool from *argv
... git-diff--tool
... GIT_EXT_DIFF=git-diff--tool-helper
... git-diff
... ... git-diff--tool-helper
... ... ... xxdiff ...

(git-diff calls itself in this scenario...)


Right now users only specify --tool=<tool> as an override.

The default behavior without --tool is:
- difftool looks up diff.tool and uses that value, or
- difftool makes a best guess based on the environ

Hmm.. if --tool supported both '--tool' and '--tool=<tool>'
then that could work.  That would make '--tool' both a switch
and an option-with-argument.  Is there anything in git that
does that?  I can imaging that being a little surprising from
a ui point of view, but it's not horrible.

What about the --no-prompt option?
Would we need that in git-diff too, or would we be able to
blindly pass it along to git-diff--tool without worrying that
git-diff would try to interpret it?

I personally like the separation of concerns --
git-diff is plumbing and git-difftool is porcelain.
But, I also agree with making it easier for users.


That said...  (off-topic)

I have a patch for difftool that lets you do stuff like this:

	$ git difftool Makefile~3
	$ git difftool Makefile~~~ Makefile~

That is *completely* the antithesis of git because git is not
file-centric.  Nonetheless, this is something people ask me
all the time and users really hate the "right" way to do it:

	$ git difftool \
		$(git log --pretty=format:%H -3 Makefile |
			tail -1) \
		-- Makefile


The point here is that since git-difftool is a frontend to
git-diff I was actually able to implement it without changing
any of the core git commands (or extend its revision syntax).
This is both good and bad.  It's good because users are much
happier using the extended file-rev syntax, and it's bad
because git-diff doesn't know about it.

What it illustrates, though, is that the separation of
concerns between the porcelain git-difftool and plumbing
git-diff is helpful specifically because it makes such things
possible.

The really cute thing, though, is that
	$ git difftool --no-ext-diff Makefile~3

...actually makes it so that git-diff understands the new
syntax.  It's quite a clever hack.  It's user-friendly
and extremely helpful, which is why I entertained the
notion of implementing it.  It basically transforms user intent
"give me Makefile from 3 changes ago back" into something that
git-diff understands, which is in my opinion the whole point of
porcelains.

That said.. it would be really user-friendly if diff and
friends understood the extended file-rev syntax directly, but
being that it overloads the '~' character and "looks" just
like a rev-parse revision specifier, I don't even know if its:

	a. possible, or
	b. something we'd want given that git is
	   commit-centric, not file-centric.

I should really start a second thread about the file-rev
syntax because I made it out of thin air and it's exactly the
kind of thing that the wisdom of the list can help vet.


An interesting thing is that users have been mailing me
directly with questions about difftool and I really want to
use the full potential of the community, which I think will
come naturally with the move out of contrib.

So.. I agree in principle but also think it wouldn't hurt to
go forward with moving git-difftool out of contrib so that we
can get more user feedback.  I also think that a scheme most
similar to (3) above seems like an interesting way to go and
would be interested to hear if it's what you envisioned.


I hope I didn't just muddy the waters further =)

Have fun,

-- 

	David

  reply	other threads:[~2009-03-21 18:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-21  7:58 What's cooking in git.git (Mar 2009, #06; Sat, 21) Junio C Hamano
2009-03-21 16:20 ` Wincent Colaiuta
2009-03-21 18:58   ` David Aguilar [this message]
2009-03-22 15:57     ` Wincent Colaiuta
2009-03-21 19:28   ` Junio C Hamano
2009-03-22 15:54     ` Wincent Colaiuta
2009-03-21 22:21 ` Johannes Sixt
2009-03-23 14:46 ` Finn Arne Gangstad
2009-03-23 16:19   ` Junio C Hamano
2009-03-24  9:02     ` Junio C Hamano
2009-03-24  9:13       ` Junio C Hamano
2009-03-24 11:16         ` Finn Arne Gangstad
2009-03-25 18:06           ` 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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090321185817.GA22540@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=win@wincent.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.
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).