Re: git-{diff,merge} refactor round 2

From: David Aguilar <davvid@gmail.com>
Date: Apr 5, 2009, 21:15 UTC
Message-ID: <20090405211533.GA1393@gmail.com>
In-Reply-To: <7vzlevv3fy.fsf@gitster.siamese.dyndns.org>

On  0, Junio C Hamano <gitster@pobox.com> wrote:

Show 34 quoted lines

> David Aguilar <davvid@gmail.com> writes:
> 
> I'll try to queue all the outstanding da/difftool patches tonight, but I
> think the patches in the series are getting to the point of needing a
> fresh redoing.  Patches like "oops, these non-user scripts should have
> been named with double-dash" can and should disappear.
> 
> Currently they are:
> 
> $ git log --oneline next..da/difftool
> 736e6b6 mergetool--lib: add new merge tool TortoiseMerge
> b3ef7cc mergetool--lib: make (g)vimdiff workable under Windows
> c4d690e mergetool--lib: consolidate the last redundant bits in {diff,merge}tool
> def88c8 mergetool-lib: specialize opendiff options when in diff mode
> bd52fab mergetool-lib: refactor run_mergetool and check_unchanged
> e87266c bash completion: add git-difftool
> 04c3b54 {diff,merge}tool: rename helpers to remove them from tab-completion
> 2a83022 mergetool-lib: add diffuse as merge and diff tool
> 73c59d9 mergetool-lib: specialize xxdiff options when in diff mode
> 273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode
> 99511d8 mergetool: use run_mergetool from git-mergetool-lib
> 37c48c7 difftool: use run_mergetool from git-mergetool-lib
> 4e314b5 mergetool-lib: introduce run_mergetool
> 588954e difftool: use valid_tool from git-mergetool-lib
> 8af4556 mergetool: use valid_tool from git-mergetool-lib
> 72286b5 difftool: use get_mergetool_path from git-mergetool-lib
> d03b97f mergetool: use get_mergetool_path from git-mergetool-lib
> c6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions
> 6108b75 mergetool: use $( ... ) instead of `backticks`
> 73786e2 difftool: add support for a difftool.prompt config variable
> 472ff62 difftool: add a -y shortcut for --no-prompt
> de2b85d difftool: use perl built-ins when testing for msys
> 9df990e difftool: add various git-difftool tests
> 8ac77f2 difftool: add git-difftool to the list of commands

It goes back even farther...

d3b8cec difftool: move 'git-difftool' out of contrib

d3db8cec is currently sitting in 'next' and is where the "oops, I should have used double-dash" lack of foresight began.

I like the *result* of what's currently sitting in da/difftool, so rewriting history is easy now that we know exactly where we're going.

I think we have a couple of options here. I won't be able to get to it until sometime tonight, but I'll throw my plan out here to get a feel for what's the better way to do it. I think we have a couple of options. I'm open to any of these:

1. Base it on the current master, completely throwing away
the existing da/difftool branch.  That would include throwing
away the commit that's in next if we really want to be clean
about the history.  In the process, move Markus' mergetool
fixes for windows to the top so that they can be applied
independently if necessary.  This series would then depend
on them.

This would probably mean a merge conflict with next at some point.

2. Base the series on next, keeping the 'move out of contrib'
patch intact.  That'll minimize conflicts but will have an
extra commit that renames difftool-helper.  I can still push
the fixes-for-windows patches up to the top so that it makes
it easier on msysgit since we already have those two patches
in the msysgit 'tortoisemerge' branch.

3. Any others?

Regardless of which it's based on, it's obvious that there'll be some squashing going on. Tentatively,

Will be squashed: 588954e difftool: use valid_tool from git-mergetool-lib 8af4556 mergetool: use valid_tool from git-mergetool-lib

Will be squashed: 72286b5 difftool: use get_mergetool_path from git-mergetool-lib d03b97f mergetool: use get_mergetool_path from git-mergetool-lib c6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions

Will be squashed: 99511d8 mergetool: use run_mergetool from git-mergetool-lib 37c48c7 difftool: use run_mergetool from git-mergetool-lib 4e314b5 mergetool-lib: introduce run_mergetool

Will be squashed: def88c8 mergetool-lib: specialize opendiff options when in diff mode 73c59d9 mergetool-lib: specialize xxdiff options when in diff mode 273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode

I'll go with whatever you guys think makes the series easiest to manage.

I think my preference would be to resend the entires series based on 'master', but I don't want to make it any harder to manage 'next'.

Thoughts?

-- 
	David