Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add a simple getpass() for MinGW
From: Johannes Schindelin @ 2009-04-08  2:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <7vfxgjub7c.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 7 Apr 2009, Junio C Hamano wrote:

> Thanks; should I take this directly or via Hannes's pull request?

Thanks for asking; Maybe Hannes has some alternative solution, so I'd like 
to have his take on things first.

Ciao,
Dscho "who really, really needs to sleep now"

^ permalink raw reply

* [ANNOUNCE] git_fast_filter
From: Elijah Newren @ 2009-04-08  3:35 UTC (permalink / raw)
  To: Git Mailing List

Just thought I'd make this available, in case there's others with
niche needs that find it useful...


git_fast_filter assists with quickly rewriting the history of a repository
by making it easy to write scripts whose purpose is to serve as safe
filters between fast-export and fast-import.  git_fast_filter comes with
example programs, a basic test-suite, and a double your money back
satisfaction guarantee.  (I love free software.)  You can get it from

  git://gitorious.org/git_fast_filter/mainline.git

In more detail...

=== Purpose ===

git_fast_filter is designed to make it easy to filter or rewrite the
history of a repository.  As such, it fills the same role as
git-filter-branch, and was written primarily to overcome the sometimes
severe speed shortcomings of git-filter-branch.  In particular, using
git_fast_filter can avoid thousands or millions of new process forks, and
can allow you to rewrite the same file only one time instead of 50,000
times.  However, while using git_fast_filter is fairly simple and quick, it
is hard to beat writing a simple git-filter-branch one-liner for efficiency
of human time.  Also, the two tools use very different methods of rewriting
history and do not have exactly overlapping feature sets, so the best tool
for a particular job is going to be very problem dependent.

As human time is often more important than computer time, especially for
one-shot rewrites, git-filter-branch will probably continue to be the more
common tool.  However, git_fast_filter is useful in cases where computer
time of a rewrite matters (particularly larger repositories and more
involved rewrites that need to be run and tested many times on large data
sets).  Also git_fast_filter has a couple features that may come in handy
in special cases (assisting with generating fast-export output from
scratch, interleaving commits from seperate repositories, and bidirectional
collaboration between filtered and unfiltered repositories).

=== Idea ===

The way git_fast_filter works is by providing a simple python library,
git_fast_filter.py.  This library can be used in simple python scripts to
create a filter for the output of git-fast-export.  Thus, the typical
calling convention is of the form:

    git fast-export | filter_script.py | git fast-import

=== Example ===

An example script that renames the 'master' branch to 'other is shown
below (this is similar to the example in the git-fast-export manpage, but
is safe against the string 'refs/heads/master' appearing in some file or
commit message in the repository):

  #!/usr/bin/python

  from git_fast_filter import Commit, FastExportFilter

  def my_commit_callback(commit):
    if commit.branch == "refs/heads/master":
      commit.branch = "refs/heads/other"

  filter = FastExportFilter(commit_callback = my_commit_callback)
  filter.run()

The user can then run this script by:
  $ mkdir target && cd target && git init
  $ (cd /PATH/LEADING/TO/source && git fast-export --all) \
       | /PATH/TO/filter_script.py | git fast-import

(Note: The user can have the script take care of the git init, the cd's,
and the invocations of git fast-export and git fast-import by just passing
directory names to FastExportFilter.run; however, writing out the details
explicitly as in the above example makes it clearer what is going on.)


Elijah

^ permalink raw reply

* Re: Any way to edit the file in index directly?
From: Ping Yin @ 2009-04-08  4:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, git mailing list
In-Reply-To: <alpine.DEB.1.00.0904080434240.10279@pacific.mpi-cbg.de>

On Wed, Apr 8, 2009 at 10:39 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Tue, 7 Apr 2009, Jeff King wrote:
>
>> On Wed, Apr 08, 2009 at 09:03:03AM +0800, Ping Yin wrote:
>>
>> > There seems to be a patch for this ( add -e?), but i forget where to
>> > find it.
>>
>> "add -p" has an "e"dit option for editing the patch. I don't recall any
>> way of directly editing the content.
>
> I posted a patch for "git add --edit", which allows you to edit the _diff_
> between the working directory and the index, and which applies the result
> using apply --recount.
>
> But that has nothing to do with "editing the index directly".
>
> But you might want to use a combination of "hash-object -w --stdin" and
> "update-index --cacheinfo".  IOW something like
>
>        sha1=$(echo Hello | git hash-object -w --stdin) &&
>        git update-index --cacheinfo 0644 $sha1 my-file
>
> However, Ping, I _strongly_ suspect an X-Y problem here.  IOW I think you
> are asking about specifics of a certain _solution_, while we probably have
> a better solution for your particular _problem_.
>

Thanks. I think 'add --edit' is just what i meant by saying 'add -e'.
I'll have it a try.

^ permalink raw reply

* Re: [RFC/PATCH] git-submodule: add support for --rebase.
From: Peter Hutterer @ 2009-04-08  4:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0904071428350.6897@intel-tinevez-2-302>

On Tue, Apr 07, 2009 at 02:38:37PM +0200, Johannes Schindelin wrote:
> Disclaimer: if you are offended by constructive criticism, or likely to
> answer with insults to the comments I offer, please stop reading this mail
> now (and please do not answer my mail, either). :-)
> 
> Still with me?  Good.  Nice to meet you.
> 
> Just for the record: responding to a patch is my strongest way of saying
> that I appreciate your work.

Thanks for the review, the updated patch is below. 
Changes:
- Commit message extended with a description of why this patch is handy.
- Less duplication in the if --rebase/else condition.
- Test case included. The test updates with and without rebase and compares
  the sha of HEAD to the sha of the master branch.

I do have a few other comments though:

> > 'git submodule update --rebase' rebases onto the current branch rather 
> > than detaching the HEAD.
> 
> I know what you want to do, but this text is wrong: it should rather read 
> something like this:
> 
> 	'git submodule update --rebase' rebases your local branch on 
> 	top of what would have been checked out to a detached HEAD 
> 	otherwise.
> 
> Maybe some of these native English speakers on this list can come up with 
> something even better ;-)

amended to your version, see below.

> > I use git-submodule to keep track of repositories that I only 
> > infrequently commit to. I keep them to have a set that is known to work.
> > 
> > git submodule update is annoying, as it (in my case needlessly) detaches the
> > HEAD, making the workflow more complicated when I do have to commit (checkout
> > master, rebase onto master, then commit).
> > 
> > This patch adds a "--rebase" flag to git submodule update that calls
> > git-rebase instead of git-checkout.
> 
> Maybe a non-first person version of this comment could go into the commit 
> message, too?  I found this highly informative.

amended, see below

> > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> > index 3b8df44..117ad3d 100644
> > --- a/Documentation/git-submodule.txt
> > +++ b/Documentation/git-submodule.txt
> > @@ -177,6 +178,12 @@ OPTIONS
> >  	This option is only valid for the update command.
> >  	Don't fetch new objects from the remote site.
> >  
> > +--rebase::
> > +	This option is only valid for the update command.
> 
> This is unnecessary, it was mentioned in the synopsis.

Correct, but in the same file the options --cached, --no-fetch, and
--summary-limit do state that they are only valid for the
update/status/summary commands, respectively. For consistency, we should
either add this sentence to --rebase, or remove them from the others. I
personally prefer having it there, just to make it more obvious.

> > +	Forward-port local commits to the index of the containing repository.
> 
> This is a bit misleading/unclear.  I'd rather have it read like this:
> 
> 	Instead of detaching the HEAD to the revision committed in the 
> 	superproject, rebase the current branch onto that revision.

Hehe. I had something like this before and then decided to copy/paste the line
from the git-rebase man page :)

Changed to: "Rebase the current branch onto the index of the containing
repository instead of detaching the HEAD." (I vaguely remember the rule that
sentences are easier to understand if you have "blah instead of foo" rather
than "instead of foo, blah")
The phrase "index of the containing repository" is commonly used in this man
page, so I think it's best to stick with it.
That better now?

> > +	If a a merge failure prevents this process, you will have to resolve
> > +	these failures with linkgit:git-rebase[1].
> > +
> >  <path>...::
> >  	Paths to submodule(s). When specified this will restrict the command
> >  	to only operate on the submodules found at the specified paths.
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 7c2e060..6180bf4 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> 
> You might want to error out when --rebase was passed with another command 
> than "update".

cmd_init(), cmd_summary(), etc. have catch-all rules for unknown options to
display the usage, so this is covered. (e.g. line 439, git-submodule.sh)

> > @@ -367,11 +372,19 @@ cmd_update()
> >  				die "Unable to fetch in submodule path '$path'"
> >  			fi
> >  
> > -			(unset GIT_DIR; cd "$path" &&
> > -				  git-checkout $force -q "$sha1") ||
> > -			die "Unable to checkout '$sha1' in submodule path '$path'"
> > +			if test -z "$rebase"
> > +			then
> > +				(unset GIT_DIR; cd "$path" &&
> > +					  git-checkout $force -q "$sha1") ||
> > +				die "Unable to checkout '$sha1' in submodule path '$path'"
> > +				say "Submodule path '$path': checked out '$sha1'"
> > +			else
> > +				(unset GIT_DIR; cd "$path" &&
> > +					git-rebase "$sha1") ||
> > +					die "Unable to rebase onto '$sha1' in submodule path '$path'"
> > +				say "Submodule path '$path': rebased onto '$sha1'"
> > +			fi
> 
> I'd actually put the "(unset GIT_DIR; cd "$path" &&" in front of the "if" 
> due to the DRY principle (Don't Repeat Yourself).
> 
> In the same spirit, I'd set a variable "action" to "checkout" or "rebase 
> onto" and munge the error/info message to use $action.
 
amended, see patch below

> Maybe you want to add a single primitive test case to make sure this 
> feature will not get broken in the future?
> 
> Other than that: nice!  very nice!
> 
> Ciao,
> Dscho
> 
> P.S.: the next patch is obvious, too: add support to specify desire to 
> rebase the submodule in .gitmodules and .git/config.
 
Cheers,
  Peter

>From b5ed1a17aa9b6042f68a546ceb3990b65d0862e0 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Fri, 27 Mar 2009 13:42:42 +1000
Subject: [PATCH] git-submodule: add support for --rebase.

'git submodule update --rebase' rebases your local branch on top of what would
have been checked out to a detached HEAD otherwise.

In some cases, detaching the HEAD when updating a submodule complicates the
workflow to commit to this submodule (checkout master, rebase, then commit).
For submodules that require frequent updates but infrequent (if any) commits,
a rebase can be executed directly by the git-submodule command, ensuring that
the submodules stay on their respective branches.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 Documentation/git-submodule.txt |   12 +++++-
 git-submodule.sh                |   22 +++++++++--
 t/t7404-submodule-update.sh     |   79 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 6 deletions(-)
 create mode 100755 t/t7404-submodule-update.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 3b8df44..7e0dcb1 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git submodule' [--quiet] add [-b branch] [--] <repository> <path>
 'git submodule' [--quiet] status [--cached] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
-'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--] [<path>...]
+'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase] [--] [<path>...]
 'git submodule' [--quiet] summary [--summary-limit <n>] [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach <command>
 'git submodule' [--quiet] sync [--] [<path>...]
@@ -113,7 +113,8 @@ init::
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached.
+	This will make the submodules HEAD be detached unless '--rebase' is
+	specified.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -177,6 +178,13 @@ OPTIONS
 	This option is only valid for the update command.
 	Don't fetch new objects from the remote site.
 
+--rebase::
+	This option is only valid for the update command.
+	Rebase the current branch onto the index of the containing repository
+	instead of detaching the HEAD.
+	If a a merge failure prevents this process, you will have to resolve
+	these failures with linkgit:git-rebase[1].
+
 <path>...::
 	Paths to submodule(s). When specified this will restrict the command
 	to only operate on the submodules found at the specified paths.
diff --git a/git-submodule.sh b/git-submodule.sh
index 7c2e060..e2d40ee 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -17,6 +17,7 @@ branch=
 quiet=
 cached=
 nofetch=
+rebase=
 
 #
 # print stuff on stdout unless -q was specified
@@ -314,6 +315,10 @@ cmd_update()
 			shift
 			nofetch=1
 			;;
+		-r|--rebase)
+			shift
+			rebase=1
+			;;
 		--)
 			shift
 			break
@@ -367,11 +372,20 @@ cmd_update()
 				die "Unable to fetch in submodule path '$path'"
 			fi
 
-			(unset GIT_DIR; cd "$path" &&
-				  git-checkout $force -q "$sha1") ||
-			die "Unable to checkout '$sha1' in submodule path '$path'"
+			if test -z "$rebase"
+			then
+				command="git-checkout $force -q"
+				action="checkout"
+				msg="checked out"
+			else
+				command="git-rebase"
+				action="rebase"
+				msg="rebased onto"
+			fi
 
-			say "Submodule path '$path': checked out '$sha1'"
+			(unset GIT_DIR; cd "$path" && $command "$sha1") ||
+			die "Unable to $action '$sha1' in submodule path '$path'"
+			say "Submodule path '$path': $msg '$sha1'"
 		fi
 	done
 }
diff --git a/t/t7404-submodule-update.sh b/t/t7404-submodule-update.sh
new file mode 100755
index 0000000..55fa291
--- /dev/null
+++ b/t/t7404-submodule-update.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Red Hat, Inc.
+#
+
+test_description='Test updating submodules
+
+This test verifies that "git submodule update" detaches the HEAD of the
+submodule and "git submodule update --rebase" does not detach the HEAD.
+'
+
+. ./test-lib.sh
+
+
+compare_head()
+{
+    sha_master=`git-rev-list --max-count=1 master`
+    sha_head=`git-rev-list --max-count=1 HEAD`
+
+    test "$sha_master" = "$sha_head"
+}
+
+
+test_expect_success 'setup a submodule tree' '
+	echo file > file &&
+	git add file &&
+	test_tick &&
+	git commit -m upstream
+	git clone . super &&
+	git clone super submodule &&
+	(cd super &&
+	 git submodule add ../submodule submodule &&
+	 test_tick &&
+	 git commit -m "submodule" &&
+	 git submodule init submodule
+	) &&
+	(cd submodule &&
+	echo "line2" > file &&
+	git add file &&
+	git commit -m "Commit 2"
+	) &&
+	(cd super &&
+ 	 (cd submodule &&
+	  git pull --rebase origin
+	 ) &&
+	 git add submodule &&
+	 git commit -m "submodule update"
+	)
+'
+
+test_expect_success 'submodule update detaching the HEAD ' '
+	(cd super/submodule &&
+	 git reset --hard HEAD~1
+	) &&
+	(cd super &&
+	 (cd submodule &&
+	  compare_head
+	 ) &&
+	 git submodule update submodule &&
+	 cd submodule &&
+	 ! compare_head
+	)
+'
+
+test_expect_success 'submodule update --rebase staying on master' '
+	(cd super/submodule &&
+	  git checkout master
+	) &&
+	(cd super &&
+	 (cd submodule &&
+	  compare_head
+	 ) &&
+	 git submodule update --rebase submodule &&
+	 cd submodule &&
+	 compare_head
+	)
+'
+
+test_done
-- 
1.6.2.2.447.g4afa7

^ permalink raw reply related

* Re: [PATCH 1/3] list-objects: add "void *data" parameter to show functions
From: Christian Couder @ 2009-04-08  4:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vk55vubcv.fsf@gitster.siamese.dyndns.org>

Le mercredi 8 avril 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > The goal of this patch is to get rid of the "static struct rev_info
> > revs" static variable in "builtin-rev-list.c".
>
> Hmm.  If it were a more library-ish file, a removal of such a static
> variable might help you to make more than one calls to a library
> function, but does it matter in files like builtin-rev-list.c?  Its
> cmd_rev_list() is like main() --- it is meant to run once and exit.
>
> So if it is the only goal of this series, I am inclined to say that I do
> not have a reason to look at the rest of the series, but as a side effect
> does this removal make some other API better?  Perhaps a more library-ish
> function is in builtin-rev-list.c and this structure should really needs
> to be passed around as a parameter, but I cannot tell solely by reading
> the goal above, without reading the patches themselves.

In the cover letter, I wrote that the patch series removes restrictions on 
using the "show_bisect_vars" function.

In fact there was a restriction on the use of the BISECT_SHOW_ALL flag 
because that would use the "show_commit" function that was using static 
variables.

The restriction was described in a comment in "bisect.h" and this comment is 
removed by the series. 

This is the relevant hunk in patch 2/3:

diff --git a/bisect.h b/bisect.h
index f5d1067..b1c334d 100644
--- a/bisect.h
+++ b/bisect.h
@@ -14,12 +14,14 @@ extern struct commit_list *filter_skipped(struct 
commit_list *list,
 #define BISECT_SHOW_TRIED      (1<<1)
 #define BISECT_SHOW_STRINGED   (1<<2)
 
-/*
- * The flag BISECT_SHOW_ALL should not be set if this function is called
- * from outside "builtin-rev-list.c" as otherwise it would use
- * static "revs" from this file.
- */
-extern int show_bisect_vars(struct rev_info *revs, int reaches, int all,
+struct rev_list_info {
+       struct rev_info *revs;
+       int show_timestamp;
+       int hdr_termination;
+       const char *header_prefix;
+};
+
+extern int show_bisect_vars(struct rev_list_info *info, int reaches, int 
all,
                            int flags);
 
 extern int bisect_next_vars(const char *prefix);


Best regards,
Christian.

> > Anyway this makes the code more clean and more generic, so it
> > should be a good thing in the long run.
>
> I wouldn't disagree with that "long run" thing, but the answer to the
> above question affects the placement of this series in my prioritized
> queue.

^ permalink raw reply related

* Re: Performance issue: initial git clone causes massive repack
From: Nicolas Pitre @ 2009-04-08  4:52 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Jakub Narebski, Sverre Rabbelier, david, Junio C Hamano,
	Nicolas Sebrecht, Robin H. Johnson, Git Mailing List
In-Reply-To: <20090407202725.GC4413@atjola.homenet>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1714 bytes --]

On Tue, 7 Apr 2009, Björn Steinbrink wrote:

> On 2009.04.07 14:56:41 -0400, Nicolas Pitre wrote:
> > On Tue, 7 Apr 2009, Björn Steinbrink wrote:
> > 
> > > On 2009.04.07 13:48:02 -0400, Nicolas Pitre wrote:
> > > > The first low hanging fruit to help this case is to make upload-pack use 
> > > > the --revs argument with pack-object to let it do the object enumeration 
> > > > itself directly, instead of relying on the rev-list output through a 
> > > > pipe.  This is what 'git repack' does already.  pack-objects has to 
> > > > access the pack anyway, so this would eliminate an extra access from a 
> > > > different process.
> > > 
> > > Hm, for an initial clone that would end up as:
> > > git pack-objects --stdout --all
> > > right?
> > > 
> > > If so, that doesn't look it it's going to work out as easily as one
> > > would hope. Robin said that both processes, git-upload-pack (which does
> > > the rev-list) and pack-objects peaked at ~2GB of RSS (which probably
> > > includes the mmapped packs). But the above pack-objects with --all peaks
> > > at 3.1G here, so it basically seems to keep all the stuff in memory that
> > > the individual processes had. But this way, it's all at once, not 2G
> > > first and then 2G in a second process, after the first one exitted.
> > 
> > Right, and it is probably faster too.
> > 
> > Can I get a copy of that repository somewhere?
> 
> http://git.overlays.gentoo.org/gitweb/?p=exp/gentoo-x86.git;a=summary
> 
> At least that's what I cloned ;-) I hope it's the right one, but it fits
> the description...

OK.  FWIW, I repacked it with --window=250 --depth=250 and obtained a 
725MB pack file.  So that's about half the originally reported size.


Nicolas

^ permalink raw reply

* [ANNOUNCE] TortoiseGit 0.5.1.0 release
From: Frank Li @ 2009-04-08  4:58 UTC (permalink / raw)
  To: tortoisegit-dev, tortoisegit-users, git, tortoisegit-announce,
	tortoisegit

32bit version: http://tortoisegit.googlecode.com/files/TortoiseGit-0.5.1.0-32bit.msi
64bit version: http://tortoisegit.googlecode.com/files/TortoiseGit-0.5.1.0-64bit.msi

Version 0.5.1.0 version(external)
Fix:
               TortoiseMerge Crash at 64bit system.

Version 0.5.0.0 version(internal)
Features:
       *Submodule Support.
        Support submodule add, sync and update.
                "Submodule Sync" is located in explore context
external menu, which need press "shift" key when right click.

       *Improve show log speed at big repository, such as git.git

       *OpenSSH can prompt password dialog

       *Clone, pull push support both OpenSSH and Plink.
        Support both key and password mode.
        Show progress when clone at git and SSH protocol.

       *Stash Save\Apply support

       *Reflog support. Need press "shift" to show reflog menu item at
context menu.

       *Add save special version to another file at file list, such as
log dailog.

       *Add external diff merge and undiff setting at settings page

       *Add Diff with workcopy at file list

       *Add MessageBox Tell user Revert Finished

       *Add Notepad2 to setup script to view text file

       *Add view in notepad2 at file list

       *Add Copy File list to clipboard

       *Choose Default SSH client when install TortoiseGit

       *Add user config and remote manage at setting dialog

       *Pull and push can autoload putty private key.


Bug Fix:
       *Fixed Issue 55:  "resolved" function doesn't delete temporary files.
       *Fix: Issue 57:  Data duplication when Clicking Check repository in
Check for modification dialog
       *Fix GetString error when edit at HistoryCombo
       *win2k context menu fix (had an issue when shift key was pressed)
       *Fix crash in logviewer on invalid time strings
       *Issue 61:  Add/Commit of files with umlauts in filename not working

^ permalink raw reply

* Re: Showing the version of a file that's in the Index.
From: Teemu Likonen @ 2009-04-08  5:00 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: Tim Visher, Git Mailing List
In-Reply-To: <200904072357.47881.markus.heidelberg@web.de>

On 2009-04-07 23:57 (+0200), Markus Heidelberg wrote:

> Teemu Likonen, 07.04.2009:
>> Or
>> 
>>     git show :file
>
> Huh, I use git-show daily for commits, but I completely forgot about it
> for files when replying.

It's nice for trees too:

    git show HEAD~3:
    git show master~1:directory/

^ permalink raw reply

* Re: [PATCH 1/3] list-objects: add "void *data" parameter to show functions
From: Junio C Hamano @ 2009-04-08  5:10 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, Johannes Schindelin
In-Reply-To: <200904080639.35499.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> This is the relevant hunk in patch 2/3:
>
> diff --git a/bisect.h b/bisect.h
> index f5d1067..b1c334d 100644
> --- a/bisect.h
> +++ b/bisect.h
> @@ -14,12 +14,14 @@ extern struct commit_list *filter_skipped(struct 
> commit_list *list,
>  #define BISECT_SHOW_TRIED      (1<<1)
>  #define BISECT_SHOW_STRINGED   (1<<2)
>  
> -/*
> - * The flag BISECT_SHOW_ALL should not be set if this function is called
> - * from outside "builtin-rev-list.c" as otherwise it would use
> - * static "revs" from this file.
> - */

Thanks for a clarification.

^ permalink raw reply

* Re: Git for Windows 1.6.2.2
From: Marius Storm-Olsen @ 2009-04-08  5:20 UTC (permalink / raw)
  To: Johannes.Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.0904080440170.10279@pacific.mpi-cbg.de>


Johannes Schindelin said the following on 08.04.2009 04:46:
> Hi,
> 
> this email tries to inform you that the Windows installer was updated to 
> the newest version of Git.
> 
> Changes since Git-1.6.2.1-preview20090322
> 
> New Features
> 
> - Comes with official git 1.6.2.2.
> - Upgraded Tcl/Tk to 8.5.5.
> - TortoiseMerge is supported by mergetool now (Thanks Markus Heidelberg).
> - Uses pthreads (faster garbage collection on multi-core machines).
> - The test suite passes!
> 
> Bugfixes
> 
> - Renaming was made more robust (due to Explorer or some virus scanners, 
>   files could not be renamed at the first try, so we have to try multiple 
>   times).
> - Johannes Sixt made lots of changes to the test-suite to identify 
>   properly which tests should pass, and which ones cannot pass due to 
>   limitations of the platform.
> - Support PAGERs with spaces in their filename.
> - Quite a few changes were undone which we needed in the olden days of 
>   msysGit.
> - Fall back to / when HOME cannot be set to the real home directory due to 
>   locale issues (works around Issue 108 for the moment).

Excellent work guys! Thanks plenty!

Oh, and go to sleep! :-p

--
.marius

^ permalink raw reply

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Markus Heidelberg @ 2009-04-08  5:33 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, charles
In-Reply-To: <1239145213-76701-1-git-send-email-davvid@gmail.com>

David Aguilar, 08.04.2009:
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh

> +guess_merge_tool () {
> +	tools="ecmerge"
> +	if merge_mode; then
> +		tools="$tools tortoisemerge"

ecmerge can now go to the block after "test -n $DISPLAY", so that in
this if-then-else really only merge-only and diff-only tools are
handled.
Then here it is
+		tools="tortoisemerge"

> +	else
> +		kompare="kompare"

and here
+		tools="kompare"

> +	fi
> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3 $kompare tkdiff $tools"
> +			tools="$tools xxdiff gvimdiff diffuse"
> +		else
> +			tools="opendiff kdiff3 $kompare tkdiff xxdiff $tools"
> +			tools="$tools meld gvimdiff diffuse"
> +		fi

And above ecmerge
And now that we remove the duplicated spaces afterwards anyway, we can
get rid of the double tools= and continue the line with \
if we adjust the sed command below...

> +	fi
> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> +		# $EDITOR is emacs so add emerge as a candidate
> +		tools="$tools emerge vimdiff"
> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> +		# $EDITOR is vim so add vimdiff as a candidate
> +		tools="$tools vimdiff emerge"
> +	else
> +		tools="$tools emerge vimdiff"
> +	fi
> +	tools="$(echo "$tools" | sed -e 's/ +/ /g')"

Doesn't work for me. For me 's/ \+/ /g' works.

...like this: 's/[ 	]\+/ /g' (space and tab)

OK, for clarification now:
	if merge_mode; then
		tools="tortoisemerge"
	else
		tools="kompare"
	fi
	if test -n "$DISPLAY"; then
		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
			tools="meld opendiff kdiff3 tkdiff xxdiff $tools \
				gvimdiff diffuse ecmerge"
		else
			tools="opendiff kdiff3 tkdiff xxdiff meld $tools \
				gvimdiff diffuse ecmerge"
		fi
	fi
	[...]
	tools="$(echo "$tools" | sed -e 's/[ 	]\+/ /g')"

> +	echo >&2 "merge tool candidates: $tools"
> +
> +	# Loop over each candidate and stop when a valid merge tool is found.
> +	for i in $tools
> +	do
> +		merge_tool_path="$(translate_merge_tool_path "$i")"
> +		if type "$merge_tool_path" > /dev/null 2>&1; then
> +			merge_tool="$i"
> +			break
> +		fi
> +	done
> +
> +	if test -z "$merge_tool" ; then
> +		echo >&2 "No known merge resolution program available."
> +		return 1
> +	fi
> +	echo "$merge_tool"
> +}

Looks good to me, after these last 2 issues are adjusted.
Maybe resend the whole series then, so that Junio can apply them easily?

Markus

^ permalink raw reply

* Re: Any way to edit the file in index directly?
From: Markus Heidelberg @ 2009-04-08  5:44 UTC (permalink / raw)
  To: Ping Yin; +Cc: Jeff King, git mailing list
In-Reply-To: <46dff0320904071927l16d54c8bv9c219e681cc96bb2@mail.gmail.com>

Ping Yin, 08.04.2009:
> On Wed, Apr 8, 2009 at 10:16 AM, Jeff King <peff@peff.net> wrote:
> > I'm not sure what you mean by "alongside".
> 
> By "aloneside" i mean i can open the index and worktree file in diff
> mode side by side for easily editing.

This is possible with git-difftool from the contrib/ area, but without
editing the index file.
But I think "worktree" is called "worktree" for a good reason.

Markus

^ permalink raw reply

* Re: [PATCH RFC 1/6] Re: send-email: Add --delay for separating emails
From: Jeff King @ 2009-04-08  6:03 UTC (permalink / raw)
  To: Nicolas Sebrecht; +Cc: Michael Witten, git
In-Reply-To: <20090407220854.GA12908@vidovic>

On Wed, Apr 08, 2009 at 12:08:54AM +0200, Nicolas Sebrecht wrote:

> > I think it may still be reasonable to implement a solution that only
> > covers some of the cases, but I what I am asking is if we know what
> > percentage of the cases that is. If we are preventing only 1% of
> > out-of-order deliveries with this, I question whether it is worth the
> > bother.
> 
> IMHO, this improvement is broken by design. We try to fix a
> receiver-only issue by a sender side fix.

I almost said the same thing: it is really the receiver's problem.
However, that doesn't mean the sender can't do simple things to help
hint the right thing to the receiver. For example, we already munge the
date fields to make sure the timestamp in each patch is increasing.

So there is precedent for giving hints to help the receiver sort the
patches. But munging the date fields is relatively transparent to the
sener. A multi-second delay is downright annoying. As a sender, I don't
think I would enable this option.

> If the receiver wants the patch series be in a good ordered _for sure_, he
> has to switch to a client mail supporting the In-Reply-To chains.

That's not enough for shallow-style patch series, like:

  PATCH 0/3
    \->PATCH 1/3
    \->PATCH 2/3
    \->PATCH 3/3

which is the proposed default for v1.6.3.

Many readers will sort by rfc822 date within a single thread level,
which is sufficient with what send-email currently generates. Sorting by
subject should also work fine.

But apparently many readers sort by date received. See this subthread:

  http://article.gmane.org/gmane.comp.version-control.git/110097

I am generally of the opinion that if it is a big problem for people,
they should get a better mail client. But I am also open to suggestions
for helping receivers on crappy mail clients as long as those
suggestions do not put a burden on the sender.

-Peff

^ permalink raw reply

* Re: [PATCH RFC 1/6] Re: send-email: Add --delay for separating emails
From: Jeff King @ 2009-04-08  6:05 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <49DBD112.5000705@op5.se>

On Wed, Apr 08, 2009 at 12:17:54AM +0200, Andreas Ericsson wrote:

> For those two reasons, I absolutely loathe deeply nested
> in-reply-to chains.

Agreed.  I don't know if you saw:

  http://article.gmane.org/gmane.comp.version-control.git/109790

a while back.

-Peff

^ permalink raw reply

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Junio C Hamano @ 2009-04-08  6:09 UTC (permalink / raw)
  To: markus.heidelberg; +Cc: David Aguilar, gitster, git, charles
In-Reply-To: <200904080733.01030.markus.heidelberg@web.de>

Markus Heidelberg <markus.heidelberg@web.de> writes:

>> +	fi
>> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
>> +		# $EDITOR is emacs so add emerge as a candidate
>> +		tools="$tools emerge vimdiff"
>> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
>> +		# $EDITOR is vim so add vimdiff as a candidate
>> +		tools="$tools vimdiff emerge"
>> +	else
>> +		tools="$tools emerge vimdiff"
>> +	fi
>> +	tools="$(echo "$tools" | sed -e 's/ +/ /g')"
>
> Doesn't work for me. For me 's/ \+/ /g' works.
> ...like this: 's/[ 	]\+/ /g' (space and tab)

Pleae don't.  "s/  */ /g' should be the most portable (the point being "do
not use one-or-more +").

> Looks good to me, after these last 2 issues are adjusted.
> Maybe resend the whole series then, so that Junio can apply them easily?

Thanks.  I've replaced the series with the following applied on top of the
'master', but I won't be merging them to 'next' for tonight, I guess.

[PATCH 01/14] doc/merge-config: list ecmerge as a built-in merge
[PATCH 02/14] git-mergetool/difftool: make (g)vimdiff workable under
[PATCH 03/14] git-mergetool: add new merge tool TortoiseMerge
[PATCH 04/14] difftool: remove merge options for opendiff, tkdiff,
[PATCH 05/14] difftool: remove the backup file feature
[PATCH 06/14] difftool: use perl built-ins when testing for msys
[PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt
[PATCH 08/14] difftool/mergetool: add diffuse as merge and diff tool
[PATCH v2 09/14] difftool: move 'git-difftool' out of contrib
[PATCH v3 10/14] difftool: add various git-difftool tests
[PATCH v2 11/14] difftool: add support for a difftool.prompt config variable
[PATCH 12/14] bash completion: add git-difftool
[PATCH 13/14] mergetool: use $( ... ) instead of `backticks`
[PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

It appears that, assuming that up to 13/14 above is what you two expected
me to pick up, we would perhaps need to only replace 14/14?

As the final sanity check, please eyeball the attached interdiff, created
this way:

 $ git checkout master^0
 $ git am -s ./+da-fourteen-patches
 $ A=$(git rev-parse HEAD)
 $ git reset --hard master
 $ git merge da/difftool ;# old series
 $ git diff --stat -p HEAD $A ;# what's new in the new series?

 Documentation/config.txt               |    3 +-
 Documentation/git-mergetool.txt        |    2 +-
 Documentation/merge-config.txt         |    6 +-
 command-list.txt                       |    1 -
 contrib/completion/git-completion.bash |    2 +-
 git-mergetool--lib.sh                  |  237 ++++++++++++++++++--------------
 t/t7800-difftool.sh                    |    4 +-
 7 files changed, 144 insertions(+), 111 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e29623..d427daf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -670,7 +670,8 @@ diff.suppressBlankEmpty::
 diff.tool::
 	Controls which diff tool is used.  `diff.tool` overrides
 	`merge.tool` when used by linkgit:git-difftool[1] and has
-	the same valid values as `merge.tool`.
+	the same valid values as `merge.tool` minus "tortoisemerge"
+	and plus "kompare".
 
 difftool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index c3a8092..ff9700d 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -27,7 +27,7 @@ OPTIONS
 	Use the merge resolution program specified by <tool>.
 	Valid merge tools are:
 	kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge,
-	diffuse and opendiff
+	diffuse, tortoisemerge and opendiff
 +
 If a merge resolution program is not specified, 'git-mergetool'
 will use the configuration variable `merge.tool`.  If the
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index c4fc3eb..4832bc7 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -23,9 +23,9 @@ merge.tool::
 	Controls which merge resolution program is used by
 	linkgit:git-mergetool[1].  Valid built-in values are: "kdiff3",
 	"tkdiff", "meld", "xxdiff", "emerge", "vimdiff", "gvimdiff",
-	"diffuse" and "opendiff".  Any other value is treated as a custom
-	merge tool and there must be a corresponding mergetool.<tool>.cmd
-	option.
+	"diffuse", "ecmerge", "tortoisemerge", and
+	"opendiff".  Any other value is treated is custom merge tool
+	and there must be a corresponding mergetool.<tool>.cmd option.
 
 merge.verbosity::
 	Controls the amount of output shown by the recursive merge
diff --git a/command-list.txt b/command-list.txt
index fd66395..fb03a2e 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -69,7 +69,6 @@ git-merge-file                          plumbingmanipulators
 git-merge-index                         plumbingmanipulators
 git-merge-one-file                      purehelpers
 git-mergetool                           ancillarymanipulators
-git-mergetool--lib                      purehelpers
 git-merge-tree                          ancillaryinterrogators
 git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6f96d75..069e19e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1192,7 +1192,7 @@ _git_mergetool ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--tool=*)
-		__gitcomp "$__git_mergetools_common" "" "${cur##--tool=}"
+		__gitcomp "$__git_mergetools_common tortoisemerge" "" "${cur##--tool=}"
 		return
 		;;
 	--*)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 3d3edda..95cc355 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,10 +1,10 @@
 # git-mergetool--lib is a library for common merge tool functions
 diff_mode() {
-	test $TOOL_MODE = "diff"
+	test "$TOOL_MODE" = diff
 }
 
 merge_mode() {
-	test $TOOL_MODE = "merge"
+	test "$TOOL_MODE" = merge
 }
 
 translate_merge_tool_path () {
@@ -30,36 +30,36 @@ translate_merge_tool_path () {
 }
 
 check_unchanged () {
-	if merge_mode; then
-		if test "$MERGED" -nt "$BACKUP"; then
-			status=0
-		else
-			while true; do
-				echo "$MERGED seems unchanged."
-				printf "Was the merge successful? [y/n] "
-				read answer < /dev/tty
-				case "$answer" in
-				y*|Y*) status=0; break ;;
-				n*|N*) status=1; break ;;
-				esac
-			done
-		fi
-	else
+	if test "$MERGED" -nt "$BACKUP"; then
 		status=0
+	else
+		while true; do
+			echo "$MERGED seems unchanged."
+			printf "Was the merge successful? [y/n] "
+			read answer < /dev/tty
+			case "$answer" in
+			y*|Y*) status=0; break ;;
+			n*|N*) status=1; break ;;
+			esac
+		done
 	fi
-	return $status
 }
 
 valid_tool () {
 	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | diffuse | tortoisemerge)
-		if test "$1" = "kompare" && ! diff_mode; then
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
 			return 1
 		fi
-		if test "$1" = "tortoisemerge" && ! merge_mode; then
+		;;
+	kompare)
+		if ! diff_mode; then
 			return 1
 		fi
-		;; # happy
+		;;
 	*)
 		if test -z "$(get_merge_tool_cmd "$1")"; then
 			return 1
@@ -79,160 +79,182 @@ get_merge_tool_cmd () {
 
 run_merge_tool () {
 	base_present="$2"
-	if diff_mode; then
-		base_present="false"
-	fi
-	if test -z "$base_present"; then
-		base_present="true"
-	fi
+	status=0
 
 	case "$1" in
 	kdiff3)
-		if $base_present; then
-			("$merge_tool_path" --auto \
-			 --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-			 -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-		else
-			if merge_mode; then
+		if merge_mode; then
+			if $base_present; then
 				("$merge_tool_path" --auto \
-				 --L1 "$MERGED (Local)" \
-				 --L2 "$MERGED (Remote)" \
-				 -o "$MERGED" "$LOCAL" "$REMOTE" \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
 				> /dev/null 2>&1)
 			else
 				("$merge_tool_path" --auto \
-				 --L1 "$MERGED (A)" \
-				 --L2 "$MERGED (B)" \
-				 "$LOCAL" "$REMOTE" \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
 				> /dev/null 2>&1)
 			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
 		fi
-		status=$?
 		;;
 	kompare)
 		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		status=$?
 		;;
 	tkdiff)
-		if $base_present; then
-			"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-		else
-			if merge_mode; then
-				"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE"
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
 			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
 		fi
-		status=$?
 		;;
 	meld)
 		if merge_mode; then
 			touch "$BACKUP"
 			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
 		else
 			"$merge_tool_path" "$LOCAL" "$REMOTE"
 		fi
-		check_unchanged
 		;;
 	diffuse)
 		if merge_mode; then
 			touch "$BACKUP"
-		fi
-		if $base_present; then
-			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" "$BASE" | cat
-		else
-			if merge_mode; then
-				"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" | cat
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
 		fi
-		check_unchanged
 		;;
 	vimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
 		fi
 		;;
 	gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
 		fi
 		;;
 	xxdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-		fi
-		if $base_present; then
-			"$merge_tool_path" -X --show-merged-pane \
-			    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-			    -R 'Accel.Search: "Ctrl+F"' \
-			    -R 'Accel.SearchForward: "Ctrl-G"' \
-			    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-		else
-			if merge_mode; then
-				"$merge_tool_path" -X $extra \
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
 					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
 					-R 'Accel.Search: "Ctrl+F"' \
 					-R 'Accel.SearchForward: "Ctrl-G"' \
-					--merged-file "$MERGED" "$LOCAL" "$REMOTE"
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
 			else
-				"$merge_tool_path" \
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
 					-R 'Accel.Search: "Ctrl+F"' \
 					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
 					"$LOCAL" "$REMOTE"
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
 		fi
-		check_unchanged
 		;;
 	opendiff)
-		merge_mode && touch "$BACKUP"
-		if $base_present; then
-			"$merge_tool_path" "$LOCAL" "$REMOTE" \
-				-ancestor "$BASE" -merge "$MERGED" | cat
-		else
-			if merge_mode; then
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
 				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
 					-merge "$MERGED" | cat
 			else
-				"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
 			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
 		fi
-		check_unchanged
 		;;
 	ecmerge)
-		merge_mode && touch "$BACKUP"
-		if $base_present; then
-			"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
-				--default --mode=merge3 --to="$MERGED"
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
 		else
 			"$merge_tool_path" "$LOCAL" "$REMOTE" \
 				--default --mode=merge2 --to="$MERGED"
 		fi
-		check_unchanged
 		;;
 	emerge)
-		if $base_present; then
-			"$merge_tool_path" -f emerge-files-with-ancestor-command \
-				"$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
 		else
 			"$merge_tool_path" -f emerge-files-command \
 				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
 		fi
-		status=$?
 		;;
 	tortoisemerge)
 		if $base_present; then
 			touch "$BACKUP"
-			"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
 			check_unchanged
 		else
 			echo "TortoiseMerge cannot be used without a base" 1>&2
@@ -240,9 +262,14 @@ run_merge_tool () {
 		fi
 		;;
 	*)
-		if test -n "$merge_tool_cmd"; then
-			if merge_mode &&
-			test "$merge_tool_trust_exit_code" = "false"; then
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
 				touch "$BACKUP"
 				( eval $merge_tool_cmd )
 				check_unchanged
@@ -250,6 +277,8 @@ run_merge_tool () {
 				( eval $merge_tool_cmd )
 				status=$?
 			fi
+		else
+			( eval $merge_tool_cmd )
 		fi
 		;;
 	esac
@@ -257,27 +286,31 @@ run_merge_tool () {
 }
 
 guess_merge_tool () {
-	if diff_mode; then
+	tools="ecmerge"
+	if merge_mode; then
+		tools="$tools tortoisemerge"
+	else
 		kompare="kompare"
 	fi
 	if test -n "$DISPLAY"; then
 		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			tools="meld kdiff3 $kompare tkdiff"
+			tools="meld opendiff kdiff3 $kompare tkdiff $tools"
 			tools="$tools xxdiff gvimdiff diffuse"
 		else
-			tools="kdiff3 $kompare tkdiff xxdiff"
+			tools="opendiff kdiff3 $kompare tkdiff xxdiff $tools"
 			tools="$tools meld gvimdiff diffuse"
 		fi
 	fi
 	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
 		# $EDITOR is emacs so add emerge as a candidate
-		tools="$tools emerge opendiff vimdiff"
+		tools="$tools emerge vimdiff"
 	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
 		# $EDITOR is vim so add vimdiff as a candidate
-		tools="$tools vimdiff opendiff emerge"
+		tools="$tools vimdiff emerge"
 	else
-		tools="$tools opendiff emerge vimdiff"
+		tools="$tools emerge vimdiff"
 	fi
+	tools="$(echo "$tools" | sed -e 's/ +/ /g')"
 	echo >&2 "merge tool candidates: $tools"
 
 	# Loop over each candidate and stop when a valid merge tool is found.
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index cbfbe87..2586f86 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -202,10 +202,10 @@ test_expect_success 'difftool + mergetool config variables' '
 
 test_expect_success 'difftool.<tool>.path' '
 	git config difftool.tkdiff.path echo &&
-	diff=$(git difftool -y -t tkdiff branch) &&
+	diff=$(git difftool --tool=tkdiff --no-prompt branch) &&
 	git config --unset difftool.tkdiff.path &&
 	lines=$(echo "$diff" | grep file | wc -l) &&
-	test "$lines" = 1
+	test "$lines" -eq 1
 '
 
 test_done

^ permalink raw reply related

* Re: [PATCH 2/5] for-each-ref: refactor refname handling
From: Junio C Hamano @ 2009-04-08  6:22 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <20090407070651.GB2924@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This code handles some special magic like *-deref and the
> :short formatting specifier. The next patch will add another
> field which outputs a ref and wants to use the same code.
>
> This patch splits the "which ref are we outputting" from the
> actual formatting. There should be no behavioral change.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> The diff is scary, but it is mostly reindentation.

... and an introduction of a bug ;-)

>  builtin-for-each-ref.c |   47 ++++++++++++++++++++++++++---------------------
>  1 files changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 4aaf75c..b50c93b 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -672,32 +672,37 @@ static void populate_value(struct refinfo *ref)
> ...
> +		/* look for "short" refname format */
> +		if (formatp) {
> +			formatp++;
> +			if (!strcmp(formatp, "short"))
> +				refname = get_short_ref(refname);
> +			else
> +				die("unknown %.*s format %s",
> +					formatp - name, name, formatp);

				die("unknown %.*s format %s",
                                    (int)(formatp - name), name, formatp);

^ permalink raw reply

* Re: [PATCH 2/5] for-each-ref: refactor refname handling
From: Jeff King @ 2009-04-08  6:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <7vprfnr7es.fsf@gitster.siamese.dyndns.org>

On Tue, Apr 07, 2009 at 11:22:51PM -0700, Junio C Hamano wrote:

> > The diff is scary, but it is mostly reindentation.
> ... and an introduction of a bug ;-)

Oops. 

> > +				die("unknown %.*s format %s",
> > +					formatp - name, name, formatp);
> 				die("unknown %.*s format %s",
>                                     (int)(formatp - name), name, formatp);

Hey, it's all 32 bits, right? ;)

Thanks for spotting it.

-Peff

^ permalink raw reply

* [PATCH v5 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: David Aguilar @ 2009-04-08  6:28 UTC (permalink / raw)
  To: gitster; +Cc: git, charles, markus.heidelberg, David Aguilar

This consolidates the common functionality from git-mergetool and
git-difftool--helper into a single git-mergetool--lib scriptlet.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

with git I learn a new portability trick everyday.
thanks all.

 .gitignore                           |    1 +
 Documentation/git-mergetool--lib.txt |   56 +++++
 Makefile                             |    1 +
 git-difftool--helper.sh              |  186 +----------------
 git-mergetool--lib.sh                |  381 ++++++++++++++++++++++++++++++++++
 git-mergetool.sh                     |  224 ++-------------------
 6 files changed, 461 insertions(+), 388 deletions(-)
 create mode 100644 Documentation/git-mergetool--lib.txt
 create mode 100644 git-mergetool--lib.sh

diff --git a/.gitignore b/.gitignore
index a36da9d..757c7f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ git-merge-recursive
 git-merge-resolve
 git-merge-subtree
 git-mergetool
+git-mergetool--lib
 git-mktag
 git-mktree
 git-name-rev
diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
new file mode 100644
index 0000000..3d57031
--- /dev/null
+++ b/Documentation/git-mergetool--lib.txt
@@ -0,0 +1,56 @@
+git-mergetool--lib(1)
+=====================
+
+NAME
+----
+git-mergetool--lib - Common git merge tool shell scriptlets
+
+SYNOPSIS
+--------
+'. "$(git --exec-path)/git-mergetool--lib"'
+
+DESCRIPTION
+-----------
+
+This is not a command the end user would want to run.  Ever.
+This documentation is meant for people who are studying the
+Porcelain-ish scripts and/or are writing new ones.
+
+The 'git-mergetool--lib' scriptlet is designed to be sourced (using
+`.`) by other shell scripts to set up functions for working
+with git merge tools.
+
+Before sourcing it, your script should set up a few variables;
+`TOOL_MODE` is used to define the operation mode for various
+functions.  'diff' and 'merge' are valid values.
+
+FUNCTIONS
+---------
+get_merge_tool::
+	returns a merge tool
+
+get_merge_tool_cmd::
+	returns the custom command for a merge tool.
+
+get_merge_tool_path::
+	returns the custom path for a merge tool.
+
+run_merge_tool::
+	launches a merge tool given the tool name and a true/false
+	flag to indicate whether a merge base is present.
+	'$merge_tool', '$merge_tool_path', and for custom commands,
+	'$merge_tool_cmd', must be defined prior to calling
+	run_merge_tool.  Additionally, '$MERGED', '$LOCAL', '$REMOTE',
+	and '$BASE' must be defined for use by the merge tool.
+
+Author
+------
+Written by David Aguilar <davvid@gmail.com>
+
+Documentation
+--------------
+Documentation by David Aguilar and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a80055f..3e56274 100644
--- a/Makefile
+++ b/Makefile
@@ -284,6 +284,7 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
+SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index f3c27d8..b450036 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -5,6 +5,10 @@
 #
 # Copyright (c) 2009 David Aguilar
 
+# Load common functions from git-mergetool--lib
+TOOL_MODE=diff
+. git-mergetool--lib
+
 # difftool.prompt controls the default prompt/no-prompt behavior
 # and is overridden with $GIT_DIFFTOOL*_PROMPT.
 should_prompt () {
@@ -16,8 +20,7 @@ should_prompt () {
 	fi
 }
 
-# This function prepares temporary files and launches the appropriate
-# merge tool.
+# Sets up shell variables and runs a merge tool
 launch_merge_tool () {
 	# Merged is the filename as it appears in the work tree
 	# Local is the contents of a/filename
@@ -37,187 +40,16 @@ launch_merge_tool () {
 	fi
 
 	# Run the appropriate merge tool command
-	case "$merge_tool" in
-	kdiff3)
-		basename=$(basename "$MERGED")
-		"$merge_tool_path" --auto \
-			--L1 "$basename (A)" \
-			--L2 "$basename (B)" \
-			"$LOCAL" "$REMOTE" \
-			> /dev/null 2>&1
-		;;
-
-	kompare)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	tkdiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	meld)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	diffuse)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	vimdiff)
-		"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
-		;;
-
-	gvimdiff)
-		"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
-		;;
-
-	xxdiff)
-		"$merge_tool_path" \
-			-R 'Accel.Search: "Ctrl+F"' \
-			-R 'Accel.SearchForward: "Ctrl-G"' \
-			"$LOCAL" "$REMOTE"
-		;;
-
-	opendiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	ecmerge)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" \
-			--default --mode=merge2 --to="$MERGED"
-		;;
-
-	emerge)
-		"$merge_tool_path" -f emerge-files-command \
-			"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-		;;
-
-	*)
-		if test -n "$merge_tool_cmd"; then
-			( eval $merge_tool_cmd )
-		fi
-		;;
-	esac
-}
-
-# Verifies that (difftool|mergetool).<tool>.cmd exists
-valid_custom_tool() {
-	merge_tool_cmd="$(git config difftool.$1.cmd)"
-	test -z "$merge_tool_cmd" &&
-	merge_tool_cmd="$(git config mergetool.$1.cmd)"
-	test -n "$merge_tool_cmd"
-}
-
-# Verifies that the chosen merge tool is properly setup.
-# Built-in merge tools are always valid.
-valid_tool() {
-	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
-		;; # happy
-	*)
-		if ! valid_custom_tool "$1"
-		then
-			return 1
-		fi
-		;;
-	esac
-}
-
-# Sets up the merge_tool_path variable.
-# This handles the difftool.<tool>.path configuration.
-# This also falls back to mergetool defaults.
-init_merge_tool_path() {
-	merge_tool_path=$(git config difftool."$1".path)
-	test -z "$merge_tool_path" &&
-	merge_tool_path=$(git config mergetool."$1".path)
-	if test -z "$merge_tool_path"; then
-		case "$1" in
-		vimdiff)
-			merge_tool_path=vim
-			;;
-		gvimdiff)
-			merge_tool_path=gvim
-			;;
-		emerge)
-			merge_tool_path=emacs
-			;;
-		*)
-			merge_tool_path="$1"
-			;;
-		esac
-	fi
+	run_merge_tool "$merge_tool"
 }
 
 # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-# If merge tool was not specified then use the diff.tool
-# configuration variable.  If that's invalid then reset merge_tool.
-# Fallback to merge.tool.
-if test -z "$merge_tool"; then
-	merge_tool=$(git config diff.tool)
-	test -z "$merge_tool" &&
-	merge_tool=$(git config merge.tool)
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-		echo >&2 "git config option diff.tool set to unknown tool: $merge_tool"
-		echo >&2 "Resetting to default..."
-		unset merge_tool
-	fi
-fi
-
-# Try to guess an appropriate merge tool if no tool has been set.
-if test -z "$merge_tool"; then
-	# We have a $DISPLAY so try some common UNIX merge tools
-	if test -n "$DISPLAY"; then
-		# If gnome then prefer meld, otherwise, prefer kdiff3 or kompare
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			merge_tool_candidates="meld kdiff3 kompare tkdiff xxdiff gvimdiff diffuse"
-		else
-			merge_tool_candidates="kdiff3 kompare tkdiff xxdiff meld gvimdiff diffuse"
-		fi
-	fi
-	if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-		# $EDITOR is emacs so add emerge as a candidate
-		merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-	elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-		# $EDITOR is vim so add vimdiff as a candidate
-		merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-	else
-		merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-	fi
-	echo "merge tool candidates: $merge_tool_candidates"
-
-	# Loop over each candidate and stop when a valid merge tool is found.
-	for i in $merge_tool_candidates
-	do
-		init_merge_tool_path $i
-		if type "$merge_tool_path" > /dev/null 2>&1; then
-			merge_tool=$i
-			break
-		fi
-	done
-
-	if test -z "$merge_tool" ; then
-		echo "No known merge resolution program available."
-		exit 1
-	fi
-
-else
-	# A merge tool has been set, so verify that it's valid.
-	if ! valid_tool "$merge_tool"; then
-		echo >&2 "Unknown merge tool $merge_tool"
-		exit 1
-	fi
-
-	init_merge_tool_path "$merge_tool"
-
-	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-		echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-		exit 1
-	fi
-fi
-
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
 
 # Launch the merge tool on each path provided by 'git diff'
 while test $# -gt 6
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
new file mode 100644
index 0000000..42ee7b4
--- /dev/null
+++ b/git-mergetool--lib.sh
@@ -0,0 +1,381 @@
+# git-mergetool--lib is a library for common merge tool functions
+diff_mode() {
+	test "$TOOL_MODE" = diff
+}
+
+merge_mode() {
+	test "$TOOL_MODE" = merge
+}
+
+translate_merge_tool_path () {
+	if test -n "$2"; then
+		echo "$2"
+	else
+		case "$1" in
+		vimdiff)
+			path=vim
+			;;
+		gvimdiff)
+			path=gvim
+			;;
+		emerge)
+			path=emacs
+			;;
+		*)
+			path="$1"
+			;;
+		esac
+		echo "$path"
+	fi
+}
+
+check_unchanged () {
+	if test "$MERGED" -nt "$BACKUP"; then
+		status=0
+	else
+		while true; do
+			echo "$MERGED seems unchanged."
+			printf "Was the merge successful? [y/n] "
+			read answer < /dev/tty
+			case "$answer" in
+			y*|Y*) status=0; break ;;
+			n*|N*) status=1; break ;;
+			esac
+		done
+	fi
+}
+
+valid_tool () {
+	case "$1" in
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
+			return 1
+		fi
+		;;
+	kompare)
+		if ! diff_mode; then
+			return 1
+		fi
+		;;
+	*)
+		if test -z "$(get_merge_tool_cmd "$1")"; then
+			return 1
+		fi
+		;;
+	esac
+}
+
+get_merge_tool_cmd () {
+	diff_mode &&
+	custom_cmd="$(git config difftool.$1.cmd)"
+	test -z "$custom_cmd" &&
+	custom_cmd="$(git config mergetool.$1.cmd)"
+	test -n "$custom_cmd" &&
+	echo "$custom_cmd"
+}
+
+run_merge_tool () {
+	base_present="$2"
+	status=0
+
+	case "$1" in
+	kdiff3)
+		if merge_mode; then
+			if $base_present; then
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			else
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		fi
+		;;
+	kompare)
+		"$merge_tool_path" "$LOCAL" "$REMOTE"
+		;;
+	tkdiff)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			else
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	meld)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	diffuse)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
+			else
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	vimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	gvimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	xxdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$REMOTE"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	opendiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
+					-merge "$MERGED" | cat
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	ecmerge)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				--default --mode=merge2 --to="$MERGED"
+		fi
+		;;
+	emerge)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" -f emerge-files-command \
+				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
+		fi
+		;;
+	tortoisemerge)
+		if $base_present; then
+			touch "$BACKUP"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+			check_unchanged
+		else
+			echo "TortoiseMerge cannot be used without a base" 1>&2
+			status=1
+		fi
+		;;
+	*)
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
+				touch "$BACKUP"
+				( eval $merge_tool_cmd )
+				check_unchanged
+			else
+				( eval $merge_tool_cmd )
+				status=$?
+			fi
+		else
+			( eval $merge_tool_cmd )
+		fi
+		;;
+	esac
+	return $status
+}
+
+guess_merge_tool () {
+	tools="ecmerge"
+	if merge_mode; then
+		tools="tortoisemerge"
+	else
+		tools="kompare"
+	fi
+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld opendiff kdiff3 tkdiff xxdiff $tools \
+				gvimdiff diffuse ecmerge"
+		else
+			tools="opendiff kdiff3 tkdiff xxdiff meld $tools \
+				gvimdiff diffuse ecmerge"
+		fi
+	fi
+	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
+		# $EDITOR is emacs so add emerge as a candidate
+		tools="$tools emerge vimdiff"
+	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
+		# $EDITOR is vim so add vimdiff as a candidate
+		tools="$tools vimdiff emerge"
+	else
+		tools="$tools emerge vimdiff"
+	fi
+	tools="$(echo "$tools" | sed -e 's/ 	*/ /g')"
+	echo >&2 "merge tool candidates: $tools"
+
+	# Loop over each candidate and stop when a valid merge tool is found.
+	for i in $tools
+	do
+		merge_tool_path="$(translate_merge_tool_path "$i")"
+		if type "$merge_tool_path" > /dev/null 2>&1; then
+			merge_tool="$i"
+			break
+		fi
+	done
+
+	if test -z "$merge_tool" ; then
+		echo >&2 "No known merge resolution program available."
+		return 1
+	fi
+	echo "$merge_tool"
+}
+
+get_configured_merge_tool () {
+	# Diff mode first tries diff.tool and falls back to merge.tool.
+	# Merge mode only checks merge.tool
+	if diff_mode; then
+		tool=$(git config diff.tool)
+	fi
+	if test -z "$tool"; then
+		tool=$(git config merge.tool)
+	fi
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
+		echo >&2 "Resetting to default..."
+		return 1
+	fi
+	echo "$tool"
+}
+
+get_merge_tool_path () {
+	# A merge tool has been set, so verify that it's valid.
+	if ! valid_tool "$merge_tool"; then
+		echo >&2 "Unknown merge tool $merge_tool"
+		exit 1
+	fi
+	if diff_mode; then
+		merge_tool_path=$(git config difftool."$merge_tool".path)
+	fi
+	if test -z "$merge_tool_path"; then
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
+	fi
+	merge_tool_path="$(translate_merge_tool_path "$merge_tool" "$merge_tool_path")"
+	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
+		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as '$merge_tool_path'"
+		exit 1
+	fi
+	echo "$merge_tool_path"
+}
+
+get_merge_tool () {
+	merge_tool="$1"
+	# Check if a merge tool has been configured
+	if test -z "$merge_tool"; then
+		merge_tool=$(get_configured_merge_tool)
+	fi
+	# Try to guess an appropriate merge tool if no tool has been set.
+	if test -z "$merge_tool"; then
+		merge_tool=$(guess_merge_tool) || exit
+	fi
+	echo "$merge_tool"
+}
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cceebb7..efa31a2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -11,7 +11,9 @@
 USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+TOOL_MODE=merge
 . git-sh-setup
+. git-mergetool--lib
 require_work_tree
 
 # Returns true if the mode reflects a symlink
@@ -110,22 +112,6 @@ resolve_deleted_merge () {
 	done
 }
 
-check_unchanged () {
-    if test "$MERGED" -nt "$BACKUP" ; then
-	status=0;
-    else
-	while true; do
-	    echo "$MERGED seems unchanged."
-	    printf "Was the merge successful? [y/n] "
-	    read answer < /dev/tty
-	    case "$answer" in
-		y*|Y*) status=0; break ;;
-		n*|N*) status=1; break ;;
-	    esac
-	done
-    fi
-}
-
 checkout_staged_file () {
     tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
 
@@ -188,107 +174,11 @@ merge_file () {
 	read ans
     fi
 
-    case "$merge_tool" in
-	kdiff3)
-	    if base_present ; then
-		("$merge_tool_path" --auto --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-		    -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    else
-		("$merge_tool_path" --auto --L1 "$MERGED (Local)" --L2 "$MERGED (Remote)" \
-		    -o "$MERGED" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    fi
-	    status=$?
-	    ;;
-	tkdiff)
-	    if base_present ; then
-		"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    else
-		"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    status=$?
-	    ;;
-	meld)
-	    touch "$BACKUP"
-	    "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	vimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	gvimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	xxdiff)
-	    touch "$BACKUP"
-	    if base_present ; then
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-	    else
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    check_unchanged
-	    ;;
-	opendiff)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED" | cat
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED" | cat
-	    fi
-	    check_unchanged
-	    ;;
-	ecmerge)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"
-	    fi
-	    check_unchanged
-	    ;;
-	emerge)
-	    if base_present ; then
-		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
-	    else
-		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-	    fi
-	    status=$?
-	    ;;
-	tortoisemerge)
-	    if base_present ; then
-		touch "$BACKUP"
-		"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
-		check_unchanged
-	    else
-		echo "TortoiseMerge cannot be used without a base" 1>&2
-		status=1
-	    fi
-	    ;;
-	*)
-	    if test -n "$merge_tool_cmd"; then
-		if test "$merge_tool_trust_exit_code" = "false"; then
-		    touch "$BACKUP"
-		    ( eval $merge_tool_cmd )
-		    check_unchanged
-		else
-		    ( eval $merge_tool_cmd )
-		    status=$?
-		fi
-	    fi
-	    ;;
-    esac
-    if test "$status" -ne 0; then
+    present=false
+    base_present &&
+    present=true
+
+    if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
 
@@ -347,44 +237,6 @@ do
     shift
 done
 
-valid_custom_tool()
-{
-    merge_tool_cmd="$(git config mergetool.$1.cmd)"
-    test -n "$merge_tool_cmd"
-}
-
-valid_tool() {
-	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | tortoisemerge)
-			;; # happy
-		*)
-			if ! valid_custom_tool "$1"; then
-				return 1
-			fi
-			;;
-	esac
-}
-
-init_merge_tool_path() {
-	merge_tool_path=$(git config mergetool.$1.path)
-	if test -z "$merge_tool_path" ; then
-		case "$1" in
-			vimdiff)
-				merge_tool_path=vim
-				;;
-			gvimdiff)
-				merge_tool_path=gvim
-				;;
-			emerge)
-				merge_tool_path=emacs
-				;;
-			*)
-				merge_tool_path=$1
-				;;
-		esac
-	fi
-}
-
 prompt_after_failed_merge() {
     while true; do
 	printf "Continue merging other unresolved paths (y/n) ? "
@@ -402,62 +254,12 @@ prompt_after_failed_merge() {
     done
 }
 
-if test -z "$merge_tool"; then
-    merge_tool=$(git config merge.tool)
-    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-	    echo >&2 "Resetting to default..."
-	    unset merge_tool
-    fi
-fi
-
-if test -z "$merge_tool" ; then
-    if test -n "$DISPLAY"; then
-        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-            merge_tool_candidates="meld kdiff3 tkdiff xxdiff tortoisemerge gvimdiff diffuse"
-        else
-            merge_tool_candidates="kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse"
-        fi
-    fi
-    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-    elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-    else
-        merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-    fi
-    echo "merge tool candidates: $merge_tool_candidates"
-    for i in $merge_tool_candidates; do
-        init_merge_tool_path $i
-        if type "$merge_tool_path" > /dev/null 2>&1; then
-            merge_tool=$i
-            break
-        fi
-    done
-    if test -z "$merge_tool" ; then
-	echo "No known merge resolution program available."
-	exit 1
-    fi
-else
-    if ! valid_tool "$merge_tool"; then
-        echo >&2 "Unknown merge_tool $merge_tool"
-        exit 1
-    fi
-
-    init_merge_tool_path "$merge_tool"
-
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
-    merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-    if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-        exit 1
-    fi
-
-    if ! test -z "$merge_tool_cmd"; then
-        merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
-    fi
-fi
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
+merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
+merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"
 
 last_status=0
 rollup_status=0
-- 
1.6.2.2.446.gfbdc

^ permalink raw reply related

* Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
From: Junio C Hamano @ 2009-04-08  6:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Sam Vilain, Frank Lichtenheld, git, Petr Baudis
In-Reply-To: <m3prfo1xh6.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Errr... wouldn't it be better to simply use 
>
> +		my ($item, $value) = split("\n", $_, 2)
>
> here? Have you tested Git::Config with a "null" value, i.e. something
> like
>
>     [section]
>         noval
>
> in the config file (which evaluates to 'true' with '--bool' option)?
> Because from what I remember from the discussion on the 
> "git config --null --list" format the lack of "\n" is used to
> distinguish between noval (which is equivalent to 'true'), and empty
> value (which is equivalent to 'false')
>
>     [boolean
>         noval        # equivalent to 'true'
>         empty1 =     # equivalent to 'false'
>         empty2 = ""  # equivalent to 'false'

I do not mind if the _write method always wrote out

	[core]
        	autocrlf = true

for a variable that is true, but it should be able to read existing

	[core]
        	autocrlf

correctly.

Sam, I think you meant to make me squash the "Oops, for no good reason,
here is a fix-up" into the previous one, but for this case, I'd appreciate
a re-roll of the series, that includes a test to read from an existing
configuration file that contains such "presense of the name alone means
boolean true" variables.

^ permalink raw reply

* Re: [PATCH 2/2] pack-objects: report actual number of threads to be used
From: Junio C Hamano @ 2009-04-08  6:30 UTC (permalink / raw)
  To: Dan McGee; +Cc: Nicolas Pitre, Jeff King, git
In-Reply-To: <alpine.LFD.2.00.0904052311020.6741@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

>> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
>> index 9fc3b35..99181fd 100644
>> --- a/builtin-pack-objects.c
>> +++ b/builtin-pack-objects.c
>> @@ -1612,7 +1612,7 @@ static void ll_find_deltas(struct object_entry
>> **list, unsigned list_size,
>>                 return;
>>         }
>>         if (progress > pack_to_stdout)
>> -               fprintf(stderr, "Delta compression using %d threads.\n",
>> +               fprintf(stderr, "Delta compression using up to %d threads.\n",
>>                                 delta_search_threads);
>> 
>>         /* Partition the work amongst work threads. */
>
> This I have absolutely no issue with.
>
> Acked-by: Nicolas Pitre <nico@cam.org>

I do not have problem with the wording either, but then the commit log
message needs to change, I think.  Care to re-submit?

^ permalink raw reply

* Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
From: Sam Vilain @ 2009-04-08  5:49 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Frank Lichtenheld, git, Petr Baudis
In-Reply-To: <m3prfo1xh6.fsf@localhost.localdomain>

Jakub Narebski wrote:
>> -		my ($item, $value) = m{(.*?)=(.*)};
>> +		my ($item, $value) = m{(.*?)\n((?s:.*))\0}
>> +			or die "failed to parse it; \$_='$_'";
> 
> Errr... wouldn't it be better to simply use 
> 
> +		my ($item, $value) = split("\n", $_, 2)
> 
> here?

Yeah, I guess that's easier to read and possibly faster; both are
using the regexp engine and using COW strings though, so it's probably
not as bad as one might think.

> Have you tested Git::Config with a "null" value, i.e. something
> like
> 
>     [section]
>         noval
> 
> in the config file (which evaluates to 'true' with '--bool' option)?
> Because from what I remember from the discussion on the 
> "git config --null --list" format the lack of "\n" is used to
> distinguish between noval (which is equivalent to 'true'), and empty
> value (which is equivalent to 'false')
> 
>     [boolean]
>         noval        # equivalent to 'true'
>         empty1 =     # equivalent to 'false'
>         empty2 = ""  # equivalent to 'false'

That I didn't consider.  Below is a patch for this.  Any more gremlins?

Subject: perl: fix no value items in Git::Config

When interpreted as boolean, items in the configuration which do not
have an '=' are interpreted as true.  Parse for this situation, and
represent it with an object in the state hash which works a bit like
undef, but isn't.  Various internal tests that items were multiple
values with ref() must become stricter.  Reported by Jakub Narebski.
Sneak a couple of vim footer changes in too.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
  pull 'perl-Config' from git://github.com/samv/git for a rebased
  version without the vim footer changes, and with cleaner whitespace.

 perl/Git/Config.pm |   36 +++++++++++++++++++++++++++---------
 t/t9700/config.t   |   22 +++++++++++++++++++++-
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/perl/Git/Config.pm b/perl/Git/Config.pm
index a35d9f3..6b4d928 100644
--- a/perl/Git/Config.pm
+++ b/perl/Git/Config.pm
@@ -144,7 +144,7 @@ sub _config {
 	}
 
 	if (defined wantarray) {
-		my @values = ref $state ? @$state :
+		my @values = ref($state) eq "ARRAY" ? @$state :
 			defined $state ? ($state) : ();
 
 		if ( my $type = $self->type( $item ) ) {
@@ -171,6 +171,8 @@ Reads the current state of the configuration file.
 
 =cut
 
+our $NOVALUE = bless [__PACKAGE__."/NOVALUE"], "Git::Config::novalue";
+
 sub read {
 	my $self = shift;
 	my $which = shift;
@@ -185,13 +187,19 @@ sub read {
 
 	local($/)="\0";
 	while (<$fh>) {
-		my ($item, $value) = m{(.*?)\n((?s:.*))\0}
-			or die "failed to parse it; \$_='$_'";
+		my ($item, $value) = split "\n", $_, 2;
+		if (defined $value) {
+			chop($value);
+		} else {
+			chop($item);
+			$value = $NOVALUE;
+		}
+		my $exists = exists $read_state->{$item};
 		my $sl = \( $read_state->{$item} );
-		if (!defined $$sl) {
+		if (!$exists) {
 			$$sl = $value;
 		}
-		elsif (!ref $$sl) {
+		elsif (!ref $$sl or ref $$sl ne "ARRAY") {
 			$$sl = [ $$sl, $value ];
 		}
 		else {
@@ -325,7 +333,7 @@ sub _write {
 		if ($type ne "string") {
 			push @cmd, "--$type";
 		}
-		if (ref $value) {
+		if (ref $value eq "ARRAY") {
 			$git->command_oneline (
 				"config", @cmd, "--replace-all",
 				 $item, $value->[0],
@@ -378,7 +386,7 @@ sub thaw {
 {
 	package Git::Config::string;
 	sub freeze { shift }
-	sub thaw   { shift }
+	sub thaw   { (shift)."" }
 }
 {
 	package Git::Config::integer;
@@ -408,6 +416,15 @@ sub thaw {
 	}
 }
 {
+	package Git::Config::novalue;
+	sub as_string { "" }
+	sub as_num    {  0 }
+	use overload
+		'""' => \&as_string,
+		'0+' => \&as_num,
+		fallback => 1;
+}
+{
 	package Git::Config::boolean;
 	our @true = qw(true yes 1);
 	our @false = qw(false no 0);
@@ -424,7 +441,8 @@ sub thaw {
 	}
 	sub thaw {
 		my $val = shift;
-		if ($val =~ m{$true_re}) {
+		if (eval{$val->isa("Git::Config::novalue")}
+			    or $val =~ m{$true_re}) {
 			1;
 		}
 		elsif ($val =~ m{$false_re}) {
@@ -464,4 +482,4 @@ Perl Artistic License 2.0 or later, or the GPL v2 or later.
 #   cperl-indent-wrt-brace: nil
 # End:
 #
-# vim: vim:tw=78:sts=0:noet
+# vim: tw=78:sts=0:noet
diff --git a/t/t9700/config.t b/t/t9700/config.t
index f0f7d2d..9d7860f 100644
--- a/t/t9700/config.t
+++ b/t/t9700/config.t
@@ -17,6 +17,14 @@ in_empty_repo sub {
 	$git->command_oneline("config", "foo.false.val", "false");
 	$git->command_oneline("config", "foo.true.val", "yes");
 	$git->command_oneline("config", "multiline.val", "hello\nmultiline.val=world");
+	open(CONFIG, ">>.git/config") or die $!;
+	print CONFIG <<CONF;
+[boolean]
+   noval
+   empty1 =
+   empty2 = ""
+CONF
+	close CONFIG;
 
 	my $conf = Git::Config->new();
 	ok($conf, "constructed a new Git::Config");
@@ -100,6 +108,18 @@ in_empty_repo sub {
 	$git->command_oneline("config", "foo.falseval", "false");
 	$git->command_oneline("config", "foo.trueval", "on");
 
+	is($conf->config("boolean.noval"), "", "noval: string");
+	is($conf->config("boolean.empty1"), "", "empty1: string");
+	is($conf->config("boolean.empty2"), "", "empty2: string");
+
+	$conf->type("boolean.*" => "boolean");
+
+	ok($conf->config("boolean.noval"), "noval: boolean");
+	eval{my $x = $conf->config("boolean.empty1")};
+	ok($@, "empty1: boolean");
+	eval{my $x = $conf->config("boolean.empty2")};
+	ok($@, "empty2: boolean");
+
 	SKIP:{
 		if (eval {
 			$git->command(
@@ -128,4 +148,4 @@ in_empty_repo sub {
 #   cperl-indent-wrt-brace: nil
 # End:
 #
-# vim: vim:tw=78:sts=0:noet
+# vim: tw=78:sts=0:noet
-- 
1.6.0

^ permalink raw reply related

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: David Aguilar @ 2009-04-08  6:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, git, charles
In-Reply-To: <7vtz4zr80v.fsf@gitster.siamese.dyndns.org>

On  0, Junio C Hamano <gitster@pobox.com> wrote:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> >> +	tools="$(echo "$tools" | sed -e 's/ +/ /g')"
> >
> > Doesn't work for me. For me 's/ \+/ /g' works.
> > ...like this: 's/[ 	]\+/ /g' (space and tab)
> 
> Pleae don't.  "s/  */ /g' should be the most portable (the point being "do
> not use one-or-more +").

v5 14/14.  good stuff.


> > Looks good to me, after these last 2 issues are adjusted.
> > Maybe resend the whole series then, so that Junio can apply them easily?
> 
> Thanks.  I've replaced the series with the following applied on top of the
> 'master', but I won't be merging them to 'next' for tonight, I guess.
> 
> ...
> [PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt
> [PATCH v2 09/14] difftool: move 'git-difftool' out of contrib
> [PATCH v3 10/14] difftool: add various git-difftool tests
> [PATCH v2 11/14] difftool: add support for a difftool.prompt config variable
> [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
> 
> It appears that, assuming that up to 13/14 above is what you two expected
> me to pick up, we would perhaps need to only replace 14/14?

Yup

> 
> As the final sanity check, please eyeball the attached interdiff, created
> this way:

That looks right to me.

> 
>  $ git checkout master^0
>  $ git am -s ./+da-fourteen-patches
>  $ A=$(git rev-parse HEAD)
>  $ git reset --hard master
>  $ git merge da/difftool ;# old series
>  $ git diff --stat -p HEAD $A ;# what's new in the new series?
> 
>  Documentation/config.txt               |    3 +-
>  Documentation/git-mergetool.txt        |    2 +-
>  Documentation/merge-config.txt         |    6 +-
>  command-list.txt                       |    1 -
>  contrib/completion/git-completion.bash |    2 +-
>  git-mergetool--lib.sh                  |  237 ++++++++++++++++++--------------
>  t/t7800-difftool.sh                    |    4 +-
>  7 files changed, 144 insertions(+), 111 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> ...

-- 

	David

^ permalink raw reply

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Charles Bailey @ 2009-04-08  6:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, David Aguilar, git
In-Reply-To: <7vtz4zr80v.fsf@gitster.siamese.dyndns.org>

On Tue, Apr 07, 2009 at 11:09:36PM -0700, Junio C Hamano wrote:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> > Looks good to me, after these last 2 issues are adjusted.
> > Maybe resend the whole series then, so that Junio can apply them easily?
> 
> Thanks.  I've replaced the series with the following applied on top of the
> 'master', but I won't be merging them to 'next' for tonight, I guess.

I feel I should be re-reviewing the mergetool patches, but while I
have sufficient time, it is usually at the weekend so I can't always
respond very rapidly.

I have been skimming this series, but most of the times that I thought
I could apply the series and have a thorough review, another comment
and re-roll has come through and I've decided to wait until it's
stable.

My slight concern is that this series has been changing very rapidly,
is everyone else happy that it is stable enough for merge into next?
Would you like my further input?

Charles.

^ permalink raw reply

* Re: [PATCH v4 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Junio C Hamano @ 2009-04-08  6:56 UTC (permalink / raw)
  To: Charles Bailey; +Cc: markus.heidelberg, David Aguilar, git
In-Reply-To: <20090408064037.GA9372@hashpling.org>

Charles Bailey <charles@hashpling.org> writes:

> I have been skimming this series, but most of the times that I thought
> I could apply the series and have a thorough review, another comment
> and re-roll has come through and I've decided to wait until it's
> stable.
>
> My slight concern is that this series has been changing very rapidly,
> is everyone else happy that it is stable enough for merge into next?
> Would you like my further input?

I'd feel safer to pick up v5 of 14/14 from David and park the result in
'pu' for a few days.  I expect myself to be overloaded with the day job
for the rest of the week and shouldn't be advancing topics without having
enough concentration myself anyway.

^ permalink raw reply

* [PATCH 1/2] Clarify the gitmodules and submodules docs
From: P Baker @ 2009-04-08  7:04 UTC (permalink / raw)
  To: git
In-Reply-To: <1239172816-38966-1-git-send-email-pbaker@retrodict.com>

Added some explanation to the docs to clear up some confusing parts of
git-submodules that appeared frequently on the mailing list.

 Signed-off-by: pbaker <pbaker@retrodict.com>
 ---

 As I dug into the reasoning and structure of git-submodule as part of
GSoC preparation, I also ran across frequently asked questions on the
mailing list. From this background, I added some explanation to the
docs to  clear up some confusing parts of git-submodules.

 - pbaker

 Documentation/git-submodule.txt |    9 ++++++---
  Documentation/gitmodules.txt    |    8 +++++++-
  2 files changed, 13 insertions(+), 4 deletions(-)

 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index 3b8df44..1ca8184 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -50,9 +50,12 @@ This command will manage the tree entries and
contents of the
  gitmodules file for you, as well as inspect the status of your
  submodules and update them.
  When adding a new submodule to the tree, the 'add' subcommand
 -is to be used.  However, when pulling a tree containing submodules,
 -these will not be checked out by default;
 -the 'init' and 'update' subcommands will maintain submodules
 +is to be used. This creates a record in the gitmodules file for each
 +submodule. When the file is committed and pulled by others it serves as an
 +in-tree reference for where to obtain the submodule.
 +
 +When pulling a tree containing submodules, these will not be checked out by
 +default; the 'init' and 'update' subcommands will maintain submodules
  checked out and at appropriate revision in your working tree.
  You can briefly inspect the up-to-date status of your submodules
  using the 'status' subcommand and get a detailed overview of the
 diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
 index d1a17e2..8f03310 100644
 --- a/Documentation/gitmodules.txt
 +++ b/Documentation/gitmodules.txt
 @@ -15,7 +15,13 @@ DESCRIPTION

  The `.gitmodules` file, located in the top-level directory of a git
  working tree, is a text file with a syntax matching the requirements
 -of linkgit:git-config[1].
 +of linkgit:git-config[1]. As this file is managed by Git, it tracks the
 +records of a project's submodules. Information stored in this file is used
 +as a hint to prime the authoritative version of the record stored in the
 +project configuration file. User specific record changes (e.g. to account
 +for differences in submodule URLs due to networking situations) should be
 +made to the configuration file, while record changes to be propagated (e.g.
 +due to a relocation of the submodule source) should be made to this file.

  The file contains one subsection per submodule, and the subsection value
  is the name of the submodule. Each submodule section also contains the
 --
 1.6.2.1.316.gedbc2

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox