Git development
 help / color / mirror / Atom feed
* Re: Filename quoting / parsing problem
From: Junio C Hamano @ 2010-01-01 20:01 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git
In-Reply-To: <7vws01li4c.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I used "cat -e" to make it easier to see that "c file " not only has SP in
> it but it has trailing space.  Let's try the result.
> 
>         $ git diff --cached | cat -e
>         diff --git "a/a\001file" "b/a\001file"$
>         new file mode 100644$
>         index 0000000..e69de29$
>         diff --git a/b file b/b file$
>         new file mode 100644$
>         index 0000000..e69de29$
>         diff --git a/c file  b/c file $
>         new file mode 100644$
>         index 0000000..e69de29$
>         $ git diff --cached >P.diff
> 
> And as you described, "b file" and "c file " are not quoted and they do
> not have ---/+++ lines.
> 
> But observe this:
> ...
> We are now back in the state without any of these files, and P.diff records
> a patch to recreate these three files, one with quoting and the other two
> without.
> 
>         $ git apply --index P.diff
>         $ git ls-files -s | cat -e
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       "a\001file"$
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       b file$
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       c file $
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       hello$
> 
> This demonstrates that The claim below is false, doesn't it?
> 
> > Not parseable:
> >     diff --git a/baz  b/baz 
> >     new file mode 100644
> >     index 0000000..e69de29
> 
> Both "b file" and "c file " are parsed by "git apply" perfectly fine.

Having said all that, I don't think we would mind a change to treat a
pathname with trailing SP a bit specially (iow, quoting "c file " in the
above failed attempt to reproduce the issue).  A pathname with SP in it is
an eyesore but is a fact of life outside of sane world, but a quoted
pathname is an even worse eyesore.  A pathname with trailing SP would be
much less common and is more likely to be corrupted by MUA and cut & paste;
with quoting we can protect them a bit better.

^ permalink raw reply

* Re: [PATCH v6 0/4] "git reset --merge" related improvements
From: Junio C Hamano @ 2010-01-01 20:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christian Couder, git, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <alpine.LFD.2.00.0912311623210.3630@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, 30 Dec 2009, Christian Couder wrote:
>>
>> Another reroll with the following changes:
>> - patches to add --keep option have been removed for now
>> - documentation patch has been moved before the core code changes
>> - commit messages have been improved
>> - tests have been much improved thanks to Junio's suggestions
>> 
>> Christian Couder (3):
>>   reset: improve mixed reset error message when in a bare repo
>>   Documentation: reset: add some tables to describe the different
>>     options
>>   reset: add a few tests for "git reset --merge"
>> 
>> Stephan Beyer (1):
>>   reset: use "unpack_trees()" directly instead of "git read-tree"
>
> FWIW, Ack on this version of the whole series - I don't think there is 
> anything controversial here, and I think avoiding the execve of read-tree 
> for something we have a library function for is a good thing (and the 
> change in behavior looks like a bug-fix to me).

The "bug-fix" needs my follow-up patch [*1*] to be a real "bug-fix", I
think.

By the way, I've always felt it somewhat confusing that "reset --merge"
implementation uses one-tree form of "read-tree -m -u", especially given
that your 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) that
describes the "bug" contrasts its behaviour with the behaviour of going to
a different branch with "checkout", that is inherently an operation that
depends on the two-tree "read-tree -m -u" semantics.

In the context of "reset", however, we don't want the check "checkout"
does between the tree that the index came from (the first tree argument,
typically HEAD) and the index before going to the target tree (the second
tree argument), so it is the right thing to do to use the one-tree form
semantics "go from the state in the index to the target tree".


[Refererence]

*1* http://article.gmane.org/gmane.comp.version-control.git/136004

^ permalink raw reply

* Re: [PATCH 1/6] run-command: add "use shell" option
From: Johannes Sixt @ 2010-01-01 22:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091230105316.GA22959@coredump.intra.peff.net>

Jeff King schrieb:
> Many callsites run "sh -c $CMD" to run $CMD. We can make it
> a little simpler for them by factoring out the munging of
> argv.
> 
> For simple cases with no arguments, this doesn't help much, but:
> 
>   1. For cases with arguments, we save the caller from
>      having to build the appropriate shell snippet.
> 
>   2. We can later optimize to avoid the shell when
>      there are no metacharacters in the program.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I made the matching tweak to the Windows half of run-command, but I
> don't actually have a box to test it on.
> 
> I modeled this after execv_git_cmd. Like that function, I try to release
> the allocated argv on error. However, we do actually leak the strbuf
> memory in one case. I'm not sure how much we care. On unix, this will
> always happen in a forked process which will either exec or die. On
> Windows, we seem to already be leaking the prepared argv for the git_cmd
> case (and now we leak the shell_cmd case, too).

That is OK. We can fix this when we find a work-load where this is
a problem.

But would you please squash this in to avoid a warning about an unused
static function on Windows.

---
  run-command.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/run-command.c b/run-command.c
index 22e2777..47ced57 100644
--- a/run-command.c
+++ b/run-command.c
@@ -48,6 +48,7 @@ static const char **prepare_shell_cmd(const char **argv)
  	return nargv;
  }

+#ifndef WIN32
  static int execv_shell_cmd(const char **argv)
  {
  	const char **nargv = prepare_shell_cmd(argv);
@@ -56,6 +57,7 @@ static int execv_shell_cmd(const char **argv)
  	free(nargv);
  	return -1;
  }
+#endif

  int start_command(struct child_process *cmd)
  {
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* [PATCH 7/6] t0021: use $SHELL_PATH for the filter script
From: Johannes Sixt @ 2010-01-01 22:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <20091230110335.GF22959@coredump.intra.peff.net>

On Windows, we need the shbang line to correctly invoke shell scripts via
a POSIX shell, except when the script is invoked via 'sh -c' because
sh (a bash) does "the right thing". Since nowadays the clean and smudge
filters are not always invoked via 'sh -c' anymore, we have to mark the
the one in t0021-conversion with #!$SHELL_PATH.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  t/t0021-conversion.sh    |    3 ++-
  t/t4030-diff-textconv.sh |    6 ++++--
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 8fc39d7..6cb8d60 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -4,7 +4,8 @@ test_description='blob conversion via gitattributes'

  . ./test-lib.sh

-cat <<\EOF >rot13.sh
+cat <<EOF >rot13.sh
+#!$SHELL_PATH
  tr \
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
    'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index c16d538..3cb7e63 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -19,10 +19,12 @@ cat >expect.text <<'EOF'
  +1
  EOF

-cat >hexdump <<'EOF'
-#!/bin/sh
+{
+	echo "#!$SHELL_PATH"
+	cat <<'EOF'
  perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1"
  EOF
+} >hexdump
  chmod +x hexdump

  test_expect_success 'setup binary file with history' '
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* [PATCH 8/6] t4030, t4031: work around bogus MSYS bash path conversion
From: Johannes Sixt @ 2010-01-01 22:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <4B3E73AE.6050003@kdbg.org>

Recall that MSYS bash converts POSIX style absolute paths to Windows style
absolute paths. Unfortunately, it converts a program argument that begins
with a double-quote and otherwise looks like an absolute POSIX path, but
in doing so, it strips everything past the second double-quote[*]. This
case is triggered in the two test scripts. The work-around is to place the
Windows style path between the quotes to avoid the path conversion.

[*] It is already bogus that a conversion is even considered when a program
argument begins with a double-quote because it cannot be an absolute POSIX
path.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  t/t4030-diff-textconv.sh       |    2 +-
  t/t4031-diff-rewrite-binary.sh |    2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3cb7e63..9b94647 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -50,7 +50,7 @@ test_expect_success 'file is considered binary by plumbing' '

  test_expect_success 'setup textconv filters' '
  	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "\"$PWD\""/hexdump &&
+	git config diff.foo.textconv "\"$(pwd)\""/hexdump &&
  	git config diff.fail.textconv false
  '

diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index 27fb31b..7e7b307 100755
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -54,7 +54,7 @@ chmod +x dump

  test_expect_success 'setup textconv' '
  	echo file diff=foo >.gitattributes &&
-	git config diff.foo.textconv "\"$PWD\""/dump
+	git config diff.foo.textconv "\"$(pwd)\""/dump
  '

  test_expect_success 'rewrite diff respects textconv' '
-- 
1.6.6.1073.gd853b.dirty

^ permalink raw reply related

* Re: [PATCH v6 4/4] reset: use "unpack_trees()" directly instead of "git read-tree"
From: Christian Couder @ 2010-01-02  5:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7vhbr6mhn9.fsf@alter.siamese.dyndns.org>

Hi and happy new year!

On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > At least disallowing means that the user _is notified_ and has to
> > manually deal with the situation.  Pretending it succeeded by resetting
> > only the index while still leaving the conflicted state in the work
> > tree intact is a bit worse in that sense.
> >
> >> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> >> index c9044c9..b40999f 100644
> >> --- a/Documentation/git-reset.txt
> >> +++ b/Documentation/git-reset.txt
> >> @@ -122,7 +122,7 @@ entries:
> >>         X       U     A    B     --soft  (disallowed)
> >>                                  --mixed  X       B     B
> >>                                  --hard   B       B     B
> >> -                                --merge (disallowed)
> >> +                                --merge  X       B     B
> >
> > IOW, I think the result should be "B B B" instead of "X B B" in this
> > case.
>
> A squashable fix-up on top of your patch to match the wish in the part
> you quoted from 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01)
> would look like this, I think.
>
> It does three things:
>
>  - Updates the documentation to match the wish of original "reset
> --merge" better, namely, "An unmerged entry is a sign that the path
> didn't have any local modification and can be safely resetted to whatever
> the new HEAD records";
>
>  - Updates read_index_unmerged(), which reads the index file into the
>    cache while dropping any higher-stage entries down to stage #0, not to
>    copy the object name recorded in the cache entry.  The code used to
>    take the object name from the highest stage entry ("theirs" if you
>    happened to have stage #3, or "ours" if they removed while you kept),
>    which essentially meant that you are getting random results and didn't
>    make sense.
>
>    The _only_ reason we want to keep a previously unmerged entry in the
>    index at stage #0 is so that we don't forget the fact that we have
>    corresponding file in the work tree in order to be able to remove it
>    when the tree we are resetting to does not have the path.  In order to
>    differentiate such an entry from ordinary cache entry, the cache entry
>    added by read_index_unmerged() records null sha1.
>
>  - Updates merged_entry() and deleted_entry() so that they pay attention
>    to cache entries with null sha1 (note that we _might_ want to use a
>    new in-core ce->ce_flags instead of using the null-sha1 hack). They
>    are previously unmerged entries, and the files in the work tree that
>    correspond to them are resetted away by oneway_merge() to the version
>    from the tree we are resetting to.
>
> Please take this with a grain of salt as I am under slight influence of
> CH3-CH2-OH while writing it, and I usually almost never drink.

I like this patch. It seems to improve the behavior of the --keep option for 
unmerged entries too.

The previous behavior was:

    working index HEAD target         working index HEAD
    ----------------------------------------------------
     X       U     A    B     --keep   X       B     B
     X       U     A    A     --keep   X       A     A

and now it is:

    working index HEAD target         working index HEAD
    ----------------------------------------------------
     X       U     A    B     --keep  (disallowed)
     X       U     A    A     --keep   X       A     A

And I think it is better, as it is more consistent with the behavior when 
there are no unmerged entries.

The only problem is that when it fails the error message is something like:

    error: Entry 'file1' would be overwritten by merge. Cannot merge.
    fatal: Could not reset index file to revision 'HEAD^'.

which is not very nice.

I will send a RFC patch series so people interested can test this.

Thanks,
Christian.

^ permalink raw reply

* [RFC/PATCH 0/5] add "--keep" option to "git reset"
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd

This patch series is sent to make it easy to test the changes in the
first patch by Junio of this series on a new "--keep" reset option.  

Christian Couder (4):
  reset: add option "--keep" to "git reset"
  reset: add test cases for "--keep" option
  Documentation: reset: describe new "--keep" option
  reset: disallow "reset --keep" outside a work tree

Junio C Hamano (1):
  reset: make "reset --merge" discard work tree changes on unmerged
    entries

 Documentation/git-reset.txt |   45 ++++++++++++++--
 builtin-reset.c             |   31 +++++++++--
 read-cache.c                |    2 +-
 t/t7103-reset-bare.sh       |   25 ++++++---
 t/t7110-reset-merge.sh      |  119 ++++++++++++++++++++++++++++++++++++++++---
 unpack-trees.c              |   19 ++++---
 6 files changed, 206 insertions(+), 35 deletions(-)

^ permalink raw reply

* [RFC/PATCH 1/5] reset: make "reset --merge" discard work tree changes on unmerged entries
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

From: Junio C Hamano <gitster@pobox.com>

Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed
"git reset --merge" when there was unmerged entries. It acknowledged
that this is not the best possible behavior, and that it would be better
if unmerged entries were reset as if --hard (instead of --merge) has
been used.

Recently another commit (reset: use "unpack_trees()" directly instead of
"git read-tree", 2009-12-30) changed the behavior of --merge to accept
resetting unmerged entries if they are reset to a different state than
HEAD, but it did not reset the changes in the work tree. So the behavior
was kind of improved, but it was not yet as if --hard has been used.

This patch finally makes "git reset --merge" behaves like
"git reset --hard" on unmerged entries.

To do that it does three things:

 - Updates the documentation to match the wish of original "reset
   --merge" better, namely, "An unmerged entry is a sign that the path
   didn't have any local modification and can be safely resetted to
   whatever the new HEAD records";

 - Updates read_index_unmerged(), which reads the index file into the
   cache while dropping any higher-stage entries down to stage #0, not
   to copy the object name recorded in the cache entry.  The code used to
   take the object name from the highest stage entry ("theirs" if you
   happened to have stage #3, or "ours" if they removed while you kept),
   which essentially meant that you are getting random results and
   didn't make sense.

   The _only_ reason we want to keep a previously unmerged entry in the
   index at stage #0 is so that we don't forget the fact that we have
   corresponding file in the work tree in order to be able to remove it
   when the tree we are resetting to does not have the path.  In order
   to differentiate such an entry from ordinary cache entry, the cache
   entry added by read_index_unmerged() records null sha1.

 - Updates merged_entry() and deleted_entry() so that they pay attention
   to cache entries with null sha1 (note that we _might_ want to use a
   new in-core ce->ce_flags instead of using the null-sha1 hack).  They
   are previously unmerged entries, and the files in the work tree that
   correspond to them are resetted away by oneway_merge() to the version
   from the tree we are resetting to.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-reset.txt |    4 ++--
 read-cache.c                |    2 +-
 t/t7110-reset-merge.sh      |   14 +++++++-------
 unpack-trees.c              |   19 ++++++++++++-------
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 2480be5..b78e2e1 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -123,14 +123,14 @@ entries:
        X       U     A    B     --soft  (disallowed)
 				--mixed  X       B     B
 				--hard   B       B     B
-				--merge  X       B     B
+				--merge  B       B     B
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
        X       U     A    A     --soft  (disallowed)
 				--mixed  X       A     A
 				--hard   A       A     A
-				--merge (disallowed)
+				--merge  A       A     A
 
 X means any state and U means an unmerged index.
 
diff --git a/read-cache.c b/read-cache.c
index 94b92d3..e07a6df 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1635,7 +1635,7 @@ int read_index_unmerged(struct index_state *istate)
 		len = strlen(ce->name);
 		size = cache_entry_size(len);
 		new_ce = xcalloc(1, size);
-		hashcpy(new_ce->sha1, ce->sha1);
+		/* don't copy sha1; this should not match anything */
 		memcpy(new_ce->name, ce->name, len);
 		new_ce->ce_flags = create_ce_flags(len, 0);
 		new_ce->ce_mode = ce->ce_mode;
diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index ff2875c..249df7c 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -135,27 +135,27 @@ test_expect_success 'setup 2 different branches' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
-# file1:     X       U     B    C     --merge  X       C     C
+# file1:     X       U     B    C     --merge  C       C     C
 test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
     test_must_fail git merge branch1 &&
-    cat file1 >orig_file1 &&
     git reset --merge HEAD^ &&
     test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
     test -z "$(git diff --cached)" &&
-    test_cmp file1 orig_file1
+    test -z "$(git diff)"
 '
 
 # The next test will test the following:
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
-# file1:     X       U     B    B     --merge  (disallowed)
-test_expect_success '"reset --merge HEAD" fails with pending merge' '
+# file1:     X       U     B    B     --merge  B       B     B
+test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     git reset --hard third &&
     test_must_fail git merge branch1 &&
-    test_must_fail git reset --merge HEAD &&
+    git reset --merge HEAD &&
     test "$(git rev-parse HEAD)" = "$(git rev-parse third)" &&
-    test -n "$(git diff --cached)"
+    test -z "$(git diff --cached)" &&
+    test -z "$(git diff)"
 '
 
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 7570475..87f95a1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -809,7 +809,11 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 {
 	int update = CE_UPDATE;
 
-	if (old) {
+	if (!old) {
+		if (verify_absent(merge, "overwritten", o))
+			return -1;
+		invalidate_ce_path(merge, o);
+	} else if (!is_null_sha1(old->sha1)) {
 		/*
 		 * See if we can re-use the old CE directly?
 		 * That way we get the uptodate stat info.
@@ -827,11 +831,12 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 				update |= CE_SKIP_WORKTREE;
 			invalidate_ce_path(old, o);
 		}
-	}
-	else {
-		if (verify_absent(merge, "overwritten", o))
-			return -1;
-		invalidate_ce_path(merge, o);
+	} else {
+		/*
+		 * Previously unmerged entry left as an existence
+		 * marker by read_index_unmerged();
+		 */
+		invalidate_ce_path(old, o);
 	}
 
 	add_entry(o, merge, update, CE_STAGEMASK);
@@ -847,7 +852,7 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,
 			return -1;
 		return 0;
 	}
-	if (verify_uptodate(old, o))
+	if (!is_null_sha1(old->sha1) && verify_uptodate(old, o))
 		return -1;
 	add_entry(o, ce, CE_REMOVE, 0);
 	invalidate_ce_path(ce, o);
-- 
1.6.6.rc2.5.g49666

^ permalink raw reply related

* [RFC/PATCH 3/5] reset: add test cases for "--keep" option
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

This shows that with the "--keep" option, changes that are both in
the work tree and the index are kept in the work tree after the
reset (but discarded in the index). As with the "--merge" option,
changes that are in both the work tree and the index are discarded
after the reset.

In the case of unmerged entries, we can see that "git reset --merge"
resets everything to the target, while with "--keep" it works only
when the target state is the same as HEAD and the work tree is not
reset.

And this shows that otherwise "--merge" and "--keep" have the same
behavior.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t7110-reset-merge.sh |  105 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 104 insertions(+), 1 deletions(-)

diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
index 249df7c..a9b2fba 100755
--- a/t/t7110-reset-merge.sh
+++ b/t/t7110-reset-merge.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2009 Christian Couder
 #
 
-test_description='Tests for "git reset --merge"'
+test_description='Tests for "git reset" with "--merge" and "--keep" options'
 
 . ./test-lib.sh
 
@@ -47,6 +47,30 @@ test_expect_success 'reset --merge is ok when switching back' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     C       C     C    D     --keep   D       D     D
+# file2:     C       D     D    D     --keep   C       D     D
+test_expect_success 'reset --keep is ok with changes in file it does not touch' '
+    git reset --hard second &&
+    cat file1 >file2 &&
+    git reset --keep HEAD^ &&
+    ! grep 4 file1 &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
+    test -z "$(git diff --cached)"
+'
+
+test_expect_success 'reset --keep is ok when switching back' '
+    git reset --keep second &&
+    grep 4 file1 &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
+    test -z "$(git diff --cached)"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     B       B     C    D     --merge  D       D     D
 # file2:     C       D     D    D     --merge  C       D     D
 test_expect_success 'reset --merge discards changes added to index (1)' '
@@ -78,6 +102,18 @@ test_expect_success 'reset --merge is ok again when switching back (1)' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     B       B     C    D     --keep   (disallowed)
+test_expect_success 'reset --keep fails with changes in index in files it touches' '
+    git reset --hard second &&
+    echo "line 5" >> file1 &&
+    git add file1 &&
+    test_must_fail git reset --keep HEAD^
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     C       C     C    D     --merge  D       D     D
 # file2:     C       C     D    D     --merge  D       D     D
 test_expect_success 'reset --merge discards changes added to index (2)' '
@@ -104,6 +140,30 @@ test_expect_success 'reset --merge is ok again when switching back (2)' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     C       C     C    D     --keep   D       D     D
+# file2:     C       C     D    D     --keep   C       D     D
+test_expect_success 'reset --keep keeps changes it does not touch' '
+    git reset --hard second &&
+    echo "line 4" >> file2 &&
+    git add file2 &&
+    git reset --keep HEAD^ &&
+    grep 4 file2 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
+    test -z "$(git diff --cached)"
+'
+
+test_expect_success 'reset --keep keeps changes when switching back' '
+    git reset --keep second &&
+    grep 4 file2 &&
+    grep 4 file1 &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse second)" &&
+    test -z "$(git diff --cached)"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     A       B     B    C     --merge  (disallowed)
 test_expect_success 'reset --merge fails with changes in file it touches' '
     git reset --hard second &&
@@ -116,6 +176,22 @@ test_expect_success 'reset --merge fails with changes in file it touches' '
     grep file1 err.log | grep "not uptodate"
 '
 
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
+# file1:     A       B     B    C     --keep   (disallowed)
+test_expect_success 'reset --keep fails with changes in file it touches' '
+    git reset --hard second &&
+    echo "line 5" >> file1 &&
+    test_tick &&
+    git commit -m "add line 5" file1 &&
+    sed -e "s/line 1/changed line 1/" <file1 >file3 &&
+    mv file3 file1 &&
+    test_must_fail git reset --keep HEAD^ 2>err.log &&
+    grep file1 err.log | grep "not uptodate"
+'
+
 test_expect_success 'setup 2 different branches' '
     git reset --hard second &&
     git branch branch1 &&
@@ -148,6 +224,18 @@ test_expect_success '"reset --merge HEAD^" is ok with pending merge' '
 #
 #           working index HEAD target         working index HEAD
 #           ----------------------------------------------------
+# file1:     X       U     B    C     --keep   (disallowed)
+test_expect_success '"reset --keep HEAD^" fails with pending merge' '
+    git reset --hard third &&
+    test_must_fail git merge branch1 &&
+    test_must_fail git reset --keep HEAD^ 2>err.log &&
+    grep file1 err.log | grep "overwritten by merge"
+'
+
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
 # file1:     X       U     B    B     --merge  B       B     B
 test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     git reset --hard third &&
@@ -158,4 +246,19 @@ test_expect_success '"reset --merge HEAD" is ok with pending merge' '
     test -z "$(git diff)"
 '
 
+# The next test will test the following:
+#
+#           working index HEAD target         working index HEAD
+#           ----------------------------------------------------
+# file1:     X       U     B    B     --keep  X       B     B
+test_expect_success '"reset --keep HEAD" is ok with pending merge' '
+    git reset --hard third &&
+    test_must_fail git merge branch1 &&
+    cat file1 >orig_file1 &&
+    git reset --keep HEAD &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse third)" &&
+    test -z "$(git diff --cached)" &&
+    test_cmp file1 orig_file1
+'
+
 test_done
-- 
1.6.6.rc2.5.g49666

^ permalink raw reply related

* [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

and give an example to show how it can be used.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-reset.txt |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index b78e2e1..3d7c206 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [--mixed | --soft | --hard | --merge] [-q] [<commit>]
+'git reset' [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]
 'git reset' [-q] [<commit>] [--] <paths>...
 'git reset' --patch [<commit>] [--] [<paths>...]
 
@@ -52,6 +52,11 @@ OPTIONS
 	and updates the files that are different between the named commit
 	and the current commit in the working tree.
 
+--keep::
+	Resets the index to match the tree recorded by the named commit,
+	but keep changes in the working tree. Aborts if the reset would
+	change files that are already changes in the working tree.
+
 -p::
 --patch::
 	Interactively select hunks in the difference between the index
@@ -86,6 +91,7 @@ reset options depending on the state of the files.
 				--mixed  A       D     D
 				--hard   D       D     D
 				--merge (disallowed)
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -93,6 +99,7 @@ reset options depending on the state of the files.
 				--mixed  A       C     C
 				--hard   C       C     C
 				--merge (disallowed)
+				--keep   A       C     C
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -100,6 +107,7 @@ reset options depending on the state of the files.
 				--mixed  B       D     D
 				--hard   D       D     D
 				--merge  D       D     D
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -107,13 +115,14 @@ reset options depending on the state of the files.
 				--mixed  B       C     C
 				--hard   C       C     C
 				--merge  C       C     C
+				--keep   B       C     C
 
 In these tables, A, B, C and D are some different states of a
 file. For example, the last line of the last table means that if a
 file is in state B in the working tree and the index, and in a
 different state C in HEAD and in the target, then "git reset
---merge target" will put the file in state C in the working tree,
-in the index and in HEAD.
+--keep target" will put the file in state B in the working tree,
+and in state C in the index and in HEAD.
 
 The following tables show what happens when there are unmerged
 entries:
@@ -124,6 +133,7 @@ entries:
 				--mixed  X       B     B
 				--hard   B       B     B
 				--merge  B       B     B
+				--keep  (disallowed)
 
       working index HEAD target         working index HEAD
       ----------------------------------------------------
@@ -131,6 +141,7 @@ entries:
 				--mixed  X       A     A
 				--hard   A       A     A
 				--merge  A       A     A
+				--keep   X       A     A
 
 X means any state and U means an unmerged index.
 
@@ -302,6 +313,30 @@ $ git add frotz.c                           <3>
 <2> This commits all other changes in the index.
 <3> Adds the file to the index again.
 
+Keep changes in working tree while discarding some previous commits::
++
+Suppose you are working on something and you commit it, and then you
+continue working a bit more, but now you think that what you have in
+your working tree should be in another branch that has nothing to do
+with what you commited previously. You can start a new branch and
+reset it while keeping the changes in your work tree.
++
+------------
+$ git tag start
+$ git branch branch1
+$ edit
+$ git commit ...                            <1>
+$ edit
+$ git branch branch2                        <2>
+$ git reset --keep start                    <3>
+------------
++
+<1> This commits your first edits in branch1.
+<2> This creates branch2, but unfortunately it contains the previous
+commit that you don't want in this branch.
+<3> This removes the unwanted previous commit, but this keeps the
+changes in your working tree.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com> and Linus Torvalds <torvalds@osdl.org>
-- 
1.6.6.rc2.5.g49666

^ permalink raw reply related

* [RFC/PATCH 2/5] reset: add option "--keep" to "git reset"
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

The purpose of this new option is to discard some of the
last commits but to keep current changes in the work tree.

The use case is when you work on something and commit
that work. And then you work on something else that touches
other files, but you don't commit it yet. Then you realize
that what you commited when you worked on the first thing
is not good or belongs to another branch.

So you want to get rid of the previous commits (at least in
the current branch) but you want to make sure that you keep
the changes you have in the work tree. And you are pretty
sure that your changes are independent from what you
previously commited, so you don't want the reset to succeed
if the previous commits changed a file that you also
changed in your work tree.

The table below shows what happens when running
"git reset --option target" to reset the HEAD to another
commit (as a special case "target" could be the same as
HEAD) in the cases where "--merge" and "--keep" behave
differently.

working index HEAD target         working index HEAD
----------------------------------------------------
  A      B     C     D   --keep    (disallowed)
                         --merge   (disallowed)
  A      B     C     C   --keep     A      C     C
                         --merge   (disallowed)
  B      B     C     D   --keep    (disallowed)
                         --merge    D      D     D
  B      B     C     C   --keep     B      C     C
                         --merge    C      C     C

In this table, A, B and C are some different states of
a file. For example the last line of the table means
that if a file is in state B in the working tree and
the index, and in a different state C in HEAD and in
the target, then "git reset --merge target" will put
the file in state C in the working tree, in the index
and in HEAD.

The following table shows what happens on unmerged entries:

working index HEAD target         working index HEAD
----------------------------------------------------
 X       U     A    B     --keep  (disallowed)
                          --merge  B       B     B
 X       U     A    A     --keep   X       A     A
                          --merge  A       A     A

In this table X can be any state and U means an unmerged
entry.

Though the error message when "reset --keep" is disallowed
on unmerged entries is something like:

error: Entry 'file1' would be overwritten by merge. Cannot merge.
fatal: Could not reset index file to revision 'HEAD^'.

which is not very nice.

A following patch will add some test cases for
"--keep", where the differences between "--merge" and
"--keep" can also be seen.

The "--keep" option is implemented by doing a 2 way merge
between HEAD and the reset target, and if this succeeds
by doing a mixed reset to the target.

The code comes from the sequencer GSoC project, where
such an option was developed by Stephan Beyer:

git://repo.or.cz/git/sbeyer.git

(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

But in the sequencer project the "reset" flag was set
in the "struct unpack_trees_options" passed to
"unpack_trees()". With this flag the changes in the
working tree were discarded if the file was different
between HEAD and the reset target.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c |   29 ++++++++++++++++++++++++-----
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 0f5022e..da61f20 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -22,13 +22,15 @@
 #include "cache-tree.h"
 
 static const char * const git_reset_usage[] = {
-	"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
+	"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]",
 	"git reset [--mixed] <commit> [--] <paths>...",
 	NULL
 };
 
-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };
-static const char *reset_type_names[] = { "mixed", "soft", "hard", "merge", NULL };
+enum reset_type { MIXED, SOFT, HARD, MERGE, KEEP, NONE };
+static const char *reset_type_names[] = {
+	"mixed", "soft", "hard", "merge", "keep", NULL
+};
 
 static char *args_to_str(const char **argv)
 {
@@ -71,6 +73,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 	if (!quiet)
 		opts.verbose_update = 1;
 	switch (reset_type) {
+	case KEEP:
 	case MERGE:
 		opts.update = 1;
 		break;
@@ -85,6 +88,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 
 	read_cache_unmerged();
 
+	if (reset_type == KEEP) {
+		unsigned char head_sha1[20];
+		if (get_sha1("HEAD", head_sha1))
+			return error("You do not have a valid HEAD.");
+		if (!fill_tree_descriptor(desc, head_sha1))
+			return error("Failed to find tree of HEAD.");
+		nr++;
+		opts.fn = twoway_merge;
+	}
+
 	if (!fill_tree_descriptor(desc + nr - 1, sha1))
 		return error("Failed to find tree of %s.", sha1_to_hex(sha1));
 	if (unpack_trees(nr, desc, &opts))
@@ -229,6 +242,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				"reset HEAD, index and working tree", HARD),
 		OPT_SET_INT(0, "merge", &reset_type,
 				"reset HEAD, index and working tree", MERGE),
+		OPT_SET_INT(0, "keep", &reset_type,
+				"reset HEAD but keep local changes", KEEP),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		OPT_END()
 	};
@@ -317,9 +332,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == SOFT) {
 		if (is_merge() || read_cache() < 0 || unmerged_cache())
 			die("Cannot do a soft reset in the middle of a merge.");
+	} else {
+		int err = reset_index_file(sha1, reset_type, quiet);
+		if (reset_type == KEEP)
+			err = err || reset_index_file(sha1, MIXED, quiet);
+		if (err)
+			die("Could not reset index file to revision '%s'.", rev);
 	}
-	else if (reset_index_file(sha1, reset_type, quiet))
-		die("Could not reset index file to revision '%s'.", rev);
 
 	/* Any resets update HEAD to the head being switched to,
 	 * saving the previous head in ORIG_HEAD before. */
-- 
1.6.6.rc2.5.g49666

^ permalink raw reply related

* [RFC/PATCH 5/5] reset: disallow "reset --keep" outside a work tree
From: Christian Couder @ 2010-01-02  5:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <20100102053303.30066.26391.chriscool@tuxfamily.org>

It is safer and consistent with "--merge" and "--hard" resets to disallow
"git reset --keep" outside a work tree.

So let's just do that and add some tests while at it.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-reset.c       |    2 +-
 t/t7103-reset-bare.sh |   25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index da61f20..52584af 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -319,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if (reset_type == HARD || reset_type == MERGE)
+	if (reset_type != SOFT && reset_type != MIXED)
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index afb55b3..1eef93c 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -11,21 +11,26 @@ test_expect_success 'setup non-bare' '
 	git commit -a -m two
 '
 
-test_expect_success 'hard reset requires a worktree' '
+test_expect_success '"hard" reset requires a worktree' '
 	(cd .git &&
 	 test_must_fail git reset --hard)
 '
 
-test_expect_success 'merge reset requires a worktree' '
+test_expect_success '"merge" reset requires a worktree' '
 	(cd .git &&
 	 test_must_fail git reset --merge)
 '
 
-test_expect_success 'mixed reset is ok' '
+test_expect_success '"keep" reset requires a worktree' '
+	(cd .git &&
+	 test_must_fail git reset --keep)
+'
+
+test_expect_success '"mixed" reset is ok' '
 	(cd .git && git reset)
 '
 
-test_expect_success 'soft reset is ok' '
+test_expect_success '"soft" reset is ok' '
 	(cd .git && git reset --soft)
 '
 
@@ -40,19 +45,23 @@ test_expect_success 'setup bare' '
 	cd bare.git
 '
 
-test_expect_success 'hard reset is not allowed in bare' '
+test_expect_success '"hard" reset is not allowed in bare' '
 	test_must_fail git reset --hard HEAD^
 '
 
-test_expect_success 'merge reset is not allowed in bare' '
+test_expect_success '"merge" reset is not allowed in bare' '
 	test_must_fail git reset --merge HEAD^
 '
 
-test_expect_success 'mixed reset is not allowed in bare' '
+test_expect_success '"keep" reset is not allowed in bare' '
+	test_must_fail git reset --keep HEAD^
+'
+
+test_expect_success '"mixed" reset is not allowed in bare' '
 	test_must_fail git reset --mixed HEAD^
 '
 
-test_expect_success 'soft reset is allowed in bare' '
+test_expect_success '"soft" reset is allowed in bare' '
 	git reset --soft HEAD^ &&
 	test "`git show --pretty=format:%s | head -n 1`" = "one"
 '
-- 
1.6.6.rc2.5.g49666

^ permalink raw reply related

* Re: [PATCH v6 2/4] Documentation: reset: add some tables to describe the different options
From: Christian Couder @ 2010-01-02  5:52 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7vd41uo16x.fsf@alter.siamese.dyndns.org>

On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > This patch adds a DISCUSSION section that contains some tables to
> > show how the different "git reset" options work depending on the
> > states of the files in the working tree, the index, HEAD and the
> > target commit.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> Much nicer.

Thanks.

> >  Documentation/git-reset.txt |   66
> > +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 66
> > insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
> > index 2d27e40..c9044c9 100644
> > --- a/Documentation/git-reset.txt
> > +++ b/Documentation/git-reset.txt
> > @@ -67,6 +67,72 @@ linkgit:git-add[1]).
> >  <commit>::
> >  	Commit to make the current HEAD. If not given defaults to HEAD.
> >
> > +DISCUSSION
> > +----------
> > +
> > +The tables below show what happens when running:
> > +
> > +----------
> > +git reset --option target
> > +----------
> > +
> > +to reset the HEAD to another commit (`target`) with the different
> > +reset options depending on the state of the files.
>
> Together with these "mechanical definitions", I think the readers would
> benefit from reading why some are disallowed.

Ok, I will add some explanations along what you write below.

> > +      working index HEAD target         working index HEAD
> > +      ----------------------------------------------------
> > +       A       B     C    D     --soft   A       B     D
> > +                                --mixed  A       D     D
> > +                                --hard   D       D     D
> > +                                --merge (disallowed)
>
> "reset --merge" is meant to be used when resetting out of a conflicted
> merge.  Because any mergy operation guarantees that the work tree file
> that is involved in the merge does not have local change wrt the index
> before it starts, and that it writes the result out to the work tree, the
> fact that we see difference between the index and the HEAD and also
> between the index and the work tree means that we are not seeing a state
> that a mergy operation left after failing with a conflict.  That is why
> we disallow --merge option in this case, and the next one.
>
> > +      working index HEAD target         working index HEAD
> > +      ----------------------------------------------------
> > +       A       B     C    C     --soft   A       B     C
> > +                                --mixed  A       C     C
> > +                                --hard   C       C     C
> > +                                --merge (disallowed)
>
> The same as above, but you are resetting to the same commit.
>
> > +      working index HEAD target         working index HEAD
> > +      ----------------------------------------------------
> > +       B       B     C    D     --soft   B       B     D
> > +                                --mixed  B       D     D
> > +                                --hard   D       D     D
> > +                                --merge  D       D     D
> >
> > +      working index HEAD target         working index HEAD
> > +      ----------------------------------------------------
> > +       B       B     C    C     --soft   B       B     C
> > +                                --mixed  B       C     C
> > +                                --hard   C       C     C
> > +                                --merge  C       C     C
>
> As this table is not only about "--merge" but to explain "reset", I think
> other interesting cases should also be covered.
>
> w=A	i=B	H=B	t=B
>
> This is "we had local change in the work tree that was unrelated to the
> merge", and "reset --merge" should be a no-op for this path.
>
> w=A	i=B	H=B	t=C
>
> This "reset --merge" is like "using checkout to switch to a commit that
> has C but we have changes in the work tree", and should fail just like
> "checkout branch" in such a situation fails without "-m" option.

Ok I will add these cases too.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH v6 3/4] reset: add a few tests for "git reset --merge"
From: Christian Couder @ 2010-01-02  5:58 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7v7hs2o16j.fsf@alter.siamese.dyndns.org>

On vendredi 01 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Commit 9e8eceab ("Add 'merge' mode to 'git reset'", 2008-12-01),
> > added the --merge option to git reset, but there were no test cases
> > for it.
> >
> > This was not a big problem because "git reset" was just forking and
> > execing "git read-tree", but this will change in a following patch.
> >
> > So let's add a few test cases to make sure that there will be no
> > regression.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> Looks good.

Thanks again.

> > +# The next test will test the following:
> > +#
> > +#           working index HEAD target         working index HEAD
> > +#           ----------------------------------------------------
> > +# file1:     C       C     C    D     --merge  D       D     D
> > +# file2:     C       D     D    D     --merge  C       D     D
> > +test_expect_success 'reset --merge is ok with changes in file it does
> > not touch' ' +    git reset --merge HEAD^ &&
> > +    ! grep 4 file1 &&
> > +    grep 4 file2 &&
> > +    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> > +    test -z "$(git diff --cached)"
> > +'
> > ...
> > +# The next test will test the following:
> > +#
> > +#           working index HEAD target         working index HEAD
> > +#           ----------------------------------------------------
> > +# file1:     C       C     C    D     --merge  D       D     D
> > +# file2:     C       C     D    D     --merge  D       D     D
> > +test_expect_success 'reset --merge discards changes added to index
> > (2)' ' +    git reset --hard second &&
> > +    echo "line 4" >> file2 &&
> > +    git add file2 &&
> > +    git reset --merge HEAD^ &&
> > +    ! grep 4 file2 &&
> > +    test "$(git rev-parse HEAD)" = "$(git rev-parse initial)" &&
> > +    test -z "$(git diff)" &&
> > +    test -z "$(git diff --cached)"
> > +'
>
> These two seem to duplicate the same case for file1; is it necessary?

No. I think I just copied the previous test and added the "git add file2" 
line.

> I am not pointing it out as something that needs to be removed; I am just
> puzzled and wondering if there is some interaction between the ways two
> paths are handled and the test is trying to check that (which I do not
> think is the case).

Best regards,
Christian.

^ permalink raw reply

* Re: [PATCH] Reword -M, when in `git log`s documention, to suggest --follow
From: Alex Vandiver @ 2010-01-02  6:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbr6phlx.fsf@alter.siamese.dyndns.org>

At Thu Dec 31 23:35:38 -0500 2009, Junio C Hamano wrote:
> [snip]

Thinking about this more, I'm more convinced that this is just a
symptom of a bigger problem -- why does the help for `git log` start
off with the _diff_ options, which do nothing unless you also use the
-p option?  It adds ~230 lines of options that are irrelevant to the
most common usage model.  It seems to me like the more correct fix
would be to move the diff options to later in the file (after the
options that are `git log`-specific), or to remove them entirely, and
replace them with a pointer to git diff's options.
 - Alex
-- 
Networking -- only one letter away from not working

^ permalink raw reply

* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Andreas Schwab @ 2010-01-02  9:06 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
	Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
	Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.76552.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> +--keep::
> +	Resets the index to match the tree recorded by the named commit,
> +	but keep changes in the working tree. Aborts if the reset would
> +	change files that are already changes in the working tree.

s/changes/changed/ in the last sentence.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: Filename quoting / parsing problem
From: Andreas Gruenbacher @ 2010-01-02 11:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr5q9lhm8.fsf@alter.siamese.dyndns.org>

On Friday 01 January 2010 09:01:19 pm Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > I used "cat -e" to make it easier to see that "c file " not only has SP
> > in it but it has trailing space.  Let's try the result.
> >
> >         $ git diff --cached | cat -e
> >         diff --git "a/a\001file" "b/a\001file"$
> >         new file mode 100644$
> >         index 0000000..e69de29$
> >         diff --git a/b file b/b file$
> >         new file mode 100644$
> >         index 0000000..e69de29$
> >         diff --git a/c file  b/c file $
> >         new file mode 100644$
> >         index 0000000..e69de29$
> >         $ git diff --cached >P.diff
> >
> > And as you described, "b file" and "c file " are not quoted and they do
> > not have ---/+++ lines.
> >
> > But observe this:
> > ...
> > We are now back in the state without any of these files, and P.diff
> > records a patch to recreate these three files, one with quoting and the
> > other two without.
> >
> >         $ git apply --index P.diff
> >         $ git ls-files -s | cat -e
> >         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0      
> > "a\001file"$ 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       b
> > file$ 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       c file $
> > 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       hello$
> >
> > This demonstrates that The claim below is false, doesn't it?
> >
> > > Not parseable:
> > >     diff --git a/baz  b/baz
> > >     new file mode 100644
> > >     index 0000000..e69de29
> >
> > Both "b file" and "c file " are parsed by "git apply" perfectly fine.

Right, the "diff --git" lines are technically still parseable when the file 
name stays the same.  With renames, lines like "diff --git a/f a/f b/f" or 
"diff --git a/f b/f b/f" are possible, but then there will also be "renamed 
from" and "renamed to" headers which will disambiguate things.  Still, it 
doesn't seem like a good idea to allow such ambiguities in the first place.

> Having said all that, I don't think we would mind a change to treat a
> pathname with trailing SP a bit specially (iow, quoting "c file " in the
> above failed attempt to reproduce the issue).

I would prefer quoting file names which contain spaces anywhere, not only at 
the end.  If quoting helps to disambiguate a program's output, I'm all for it.  
People who can't be bothered with such details can always use a pretty printer 
(side by side view, whatever).

Thanks,
Andreas

^ permalink raw reply

* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Nguyen Thai Ngoc Duy @ 2010-01-02 11:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl4zy5z3.fsf@alter.siamese.dyndns.org>

On Wed, Dec 30, 2009 at 11:09:52PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > This looks a bit wrong for a couple of reasons:
> >
> >  - external_grep() is designed to return negative without running external
> >    grep when it shouldn't be used (see the beginning of the function for
> >    how it refuses to run when opt->extended is set and other conditions).
> >    The new logic seems to belong there, i.e. "in addition to the existing
> >    case we decline, if ce_skip_worktree() entry exists in the cache, we
> >    decline";
> 
> IOW, something like this instead of your patch.  You would want to tests
> to demonstrate the original breakage, perhaps?

Your patch works great. By the way I think we should move "cached"
check from grep_cache() into external_grep() too, for consistency.

I thought of tests when I wrote the patch, but it was hard to find a
reliable way to detect if a git build supports external grep. Perhaps
this on top of yours? The way to check for external grep support isn't
nice, but it works.


diff --git a/builtin-grep.c b/builtin-grep.c
index f093b60..bc4500a 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -222,6 +222,7 @@ static int exec_grep(int argc, const char **argv)
 	int status;
 
 	argv[argc] = NULL;
+	trace_argv_printf(argv, "trace: grep:");
 	pid = fork();
 	if (pid < 0)
 		return pid;
@@ -371,7 +372,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 	struct grep_pat *p;
 
 	if (opt->extended || (opt->relative && opt->prefix_length)
-	    || has_skip_worktree_entry(opt, paths))
+	    || cached || has_skip_worktree_entry(opt, paths))
 		return -1;
 	len = nr = 0;
 	push_arg("grep");
@@ -509,7 +510,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
 	 * we grep through the checked-out files. It tends to
 	 * be a lot more optimized
 	 */
-	if (!cached && external_grep_allowed) {
+	if (external_grep_allowed) {
 		hit = external_grep(opt, paths, cached);
 		if (hit >= 0)
 			return hit;
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index abd14bf..f77970c 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -8,6 +8,14 @@ test_description='git grep various.
 
 . ./test-lib.sh
 
+support_external_grep() {
+	case "$(git grep -h 2>&1 >/dev/null|grep -e --ext-grep)" in
+	*"(default)"*)  return 0;;
+	*"(ignored by this build)"*) return 1;;
+	*) test_expect_success 'External grep check is broken' 'false';;
+	esac
+}
+
 cat >hello.c <<EOF
 #include <stdio.h>
 int main(int argc, const char **argv)
@@ -426,4 +434,20 @@ test_expect_success 'grep -Fi' '
 	test_cmp expected actual
 '
 
+if support_external_grep; then
+
+test_expect_success 'external grep' '
+	GIT_TRACE=2 git grep foo >/dev/null 2>actual &&
+	grep "trace: grep:.*foo" actual >/dev/null
+'
+test_expect_success 'no external grep when skip-worktree entries exist' '
+	git update-index --skip-worktree file &&
+	GIT_TRACE=2 git grep foo >/dev/null 2>actual &&
+	! grep "trace: grep:" actual >/dev/null &&
+	git update-index --no-skip-worktree file
+'
+
+else
+	say "External grep tests skipped"
+fi
 test_done

-- 
Duy

^ permalink raw reply related

* User-wide Git config directory (was Re: [PATCH v2] Let core.excludesfile default to ~/.gitexcludes.)
From: Matthieu Moy @ 2010-01-02 12:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vk4w4z1h6.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Nanako Shiraishi <nanako3@lavabit.com> writes:
>>
>>> Junio, could you tell us what happened to this thread?
>>
>> I'm not Junio, but I can try ...
>> ...
>> So, I dropped the patch until we get closer to a consensus on what the
>> default value should be.
>
> Thanks for status updates.  Should we start trying to reach a consensus on
> how to move forward, or is it not your itch?

I wouldn't say it's not my itch. But I'm not sure I'm the best person
to do the job (job = comming close to a consensus, not writting the
code, which shouldn't be hard). Still, I can at least participate ;-).

> It is not _too_ bad to treat ~/.gitconfig specially and support reading
> from ~/.$SOMEGITTTYNAME/{excludes,attributes} files.

Let's throw in some ideas about what ~/$SOMEGITTTYNAME/ could be :

1) ~/.git/ => no, that would clash with people versioning their $HOME.

2) ~/.gitconfig/ => no, there's already a file ~/.gitconfig

3) ~/.gitglobal/ => that's an option.

4) ~/.config/git/ or, if set, $XDG_CONFIG_HOME/git/ (see
   http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html )
   => my prefered one, already discussed in a different context here :
   http://thread.gmane.org/gmane.comp.version-control.git/135447/focus=135559

Then, there's the question of what to put in this directory. I can see
two and a half options:

a) ~/.$SOMEGITTTYNAME/{excludes,attributes} as you propose.

a') ~/.$SOMEGITTTYNAME/{ignore,attributes} => I think "ignore" is the
   advertized vocabulary most of the time in porcelain, and "excludes"
   exists mostly for historical reasons.

b) ~/.$SOMEGITTTYNAME/<same thing as the content of $GIT_DIR>, that
   is:
   ~/.$SOMEGITTTYNAME/info/exclude
   ~/.$SOMEGITTTYNAME/info/attributes
   and perhaps
   ~/.$SOMEGITTTYNAME/config
   we could imagine also
   ~/.$SOMEGITTTYNAME/hooks

If I were really happy with the layout of $GIT_DIR, I'd vote for b)
for consistancy. But I'm not _that_ happy with it: why are exclude and
attributes not in the same directory as config? why is exclude
singular and attributes plural?

So, I dunno.

> We can also add support for ~/.$SOMEGITTYNAME/config and try reading
> from it as well, but even if we did so I don't think we should stop
> reading from ~/.gitconfig.

Yes, people having a ~/.gitconfig around should be able to continue to
use it (for years at least, and most likely forever).

And you're right to present it as a different problem. Perhaps we
should solve the problem above before starting debating about this
one.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* RFC: display dirty submodule working directory in git gui and gitk
From: Jens Lehmann @ 2010-01-02 15:33 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Shawn O. Pearce,
	Paul Mackerras, Heiko Voigt, Lars Hjemli

Now that we have much better output when displaying diffs of
submodules in git gui and gitk (many thanks to all involved!),
another usability issue shows up: A dirty working directory of
a submodule isn't visible in git gui or gitk.

So you might think a "submodule update" would be ok - as you
see no changes - just too see it fail because the submodules
working directory is dirty.

Or - even worse - you /think/ you committed your changes in
a submodule while you didn't. That can lead to 'interesting'
problems which can be pretty hard to diagnose (like breaking
builds on other peoples machines).


A possible solution could look like this:

AFAICS, git gui and gitk use "git diff-files" both to get the
file names of unstaged local changes and to later display the
actual differences.

If they could tell the diff core to also check the submodule
working directories and to output an extra line - maybe
something like "Submodule <name> contains uncommitted local
changes" - when a submodules working directory is dirty,
git gui and gitk could show the submodules state adequately.


What do you think about this approach?

^ permalink raw reply

* [PATCH] stash: mention --patch in usage string.
From: Matthieu Moy @ 2010-01-02 16:35 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy


Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-stash.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f796c2f..3a0685f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE="list [<options>]
    or: $dashless drop [-q|--quiet] [<stash>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
-   or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
+   or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
-- 
1.6.6.76.gd6b23.dirty

^ permalink raw reply related

* [PATCH 0/2] git-gui: Add a special diff popup menu for submodules
From: Jens Lehmann @ 2010-01-02 16:57 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List

The first patch is a resend (first sent on December 7th 2009) and
necessary for the second patch to work properly.

Jens Lehmann (2):
  git-gui: Unstaging a partly staged entry didn't update file_states
    correctly
  git-gui: Add a special diff popup menu for submodules

 git-gui/git-gui.sh |  130 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 113 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH 1/2] git-gui: Unstaging a partly staged entry didn't update file_states correctly
From: Jens Lehmann @ 2010-01-02 16:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <4B3F7AE2.10007@web.de>

When unstaging a partly staged file or submodule, the file_states list
was not updated properly (unless unstaged linewise). Its index_info part
did not contain the former head_info as it should have.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-gui/git-gui.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 718277a..e0dd5b5 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1613,6 +1613,9 @@ proc merge_state {path new_state {head_info {}} {index_info {}}} {
 	} elseif {$s0 ne {_} && [string index $state 0] eq {_}
 		&& $head_info eq {}} {
 		set head_info $index_info
+	} elseif {$s0 eq {_} && [string index $state 0] ne {_}} {
+		set index_info $head_info
+		set head_info {}
 	}

 	set file_states($path) [list $s0$s1 $icon \
-- 
1.6.6.339.g7cacc

^ permalink raw reply related

* [PATCH 2/2] git-gui: Add a special diff popup menu for submodules
From: Jens Lehmann @ 2010-01-02 16:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <4B3F7AE2.10007@web.de>

To make it easier for users to deal with submodules, a special diff popup
menu has been added for submodules. The "Show Less Context" and "Show More
Context" entries have been removed, as they don't make any sense for a
submodule summary. Four new entries are added to the top of the popup menu
to gain access to more detailed information about the changes in a
submodule than the plain summary does offer. These are:
- "Visualize These Changes In The Submodule"
  starts gitk showing the selected commit range
- "Visualize These Changes In The Submodule"
  starts gitk showing the whole submodule history of the current branch
- "Visualize All Branch History In The Submodule"
  starts gitk --all in the submodule
- "Start git gui In The Submodule"
  guess what :-)

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-gui/git-gui.sh |  127 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index e0dd5b5..ee80d7d 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1923,7 +1923,9 @@ proc incr_font_size {font {amt 1}} {

 set starting_gitk_msg [mc "Starting gitk... please wait..."]

-proc do_gitk {revs} {
+proc do_gitk {revs {is_submodule false}} {
+	global current_diff_path file_states current_diff_side ui_index
+
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
 	#
@@ -1941,14 +1943,72 @@ proc do_gitk {revs} {
 		}

 		set pwd [pwd]
-		cd [file dirname [gitdir]]
-		set env(GIT_DIR) [file tail [gitdir]]
-
+		if {!$is_submodule} {
+			cd [file dirname [gitdir]]
+			set env(GIT_DIR) [file tail [gitdir]]
+		} else {
+			cd $current_diff_path
+			if {$revs eq {--}} {
+				set s $file_states($current_diff_path)
+				set old_sha1 {}
+				set new_sha1 {}
+				switch -glob -- [lindex $s 0] {
+				M_ { set old_sha1 [lindex [lindex $s 2] 1] }
+				_M { set old_sha1 [lindex [lindex $s 3] 1] }
+				MM {
+					if {$current_diff_side eq $ui_index} {
+						set old_sha1 [lindex [lindex $s 2] 1]
+						set new_sha1 [lindex [lindex $s 3] 1]
+					} else {
+						set old_sha1 [lindex [lindex $s 3] 1]
+					}
+				}
+				}
+				set revs $old_sha1...$new_sha1
+			}
+			if {[info exists env(GIT_DIR)]} {
+				unset env(GIT_DIR)
+			}
+		}
 		eval exec $cmd $revs &

-		if {$old_GIT_DIR eq {}} {
+		if {$old_GIT_DIR ne {}} {
+			set env(GIT_DIR) $old_GIT_DIR
+		}
+		cd $pwd
+
+		ui_status $::starting_gitk_msg
+		after 10000 {
+			ui_ready $starting_gitk_msg
+		}
+	}
+}
+
+proc do_git_gui {} {
+	global current_diff_path
+
+	# -- Always start git gui through whatever we were loaded with.  This
+	#    lets us bypass using shell process on Windows systems.
+	#
+	set exe [_which git]
+	if {$exe eq {}} {
+		error_popup [mc "Couldn't find git gui in PATH"]
+	} else {
+		global env
+
+		if {[info exists env(GIT_DIR)]} {
+			set old_GIT_DIR $env(GIT_DIR)
 			unset env(GIT_DIR)
 		} else {
+			set old_GIT_DIR {}
+		}
+
+		set pwd [pwd]
+		cd $current_diff_path
+
+		eval exec $exe gui &
+
+		if {$old_GIT_DIR ne {}} {
 			set env(GIT_DIR) $old_GIT_DIR
 		}
 		cd $pwd
@@ -3143,15 +3203,6 @@ $ui_diff tag raise sel

 proc create_common_diff_popup {ctxm} {
 	$ctxm add command \
-		-label [mc "Show Less Context"] \
-		-command show_less_context
-	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-	$ctxm add command \
-		-label [mc "Show More Context"] \
-		-command show_more_context
-	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
-	$ctxm add separator
-	$ctxm add command \
 		-label [mc Refresh] \
 		-command reshow_diff
 	lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
@@ -3206,6 +3257,15 @@ $ctxm add command \
 set ui_diff_applyline [$ctxm index last]
 lappend diff_actions [list $ctxm entryconf $ui_diff_applyline -state]
 $ctxm add separator
+$ctxm add command \
+	-label [mc "Show Less Context"] \
+	-command show_less_context
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add command \
+	-label [mc "Show More Context"] \
+	-command show_more_context
+lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
+$ctxm add separator
 create_common_diff_popup $ctxm

 set ctxmmg .vpane.lower.diff.body.ctxmmg
@@ -3228,9 +3288,40 @@ $ctxmmg add command \
 	-command {merge_resolve_one 1}
 lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
 $ctxmmg add separator
+$ctxmmg add command \
+	-label [mc "Show Less Context"] \
+	-command show_less_context
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add command \
+	-label [mc "Show More Context"] \
+	-command show_more_context
+lappend diff_actions [list $ctxmmg entryconf [$ctxmmg index last] -state]
+$ctxmmg add separator
 create_common_diff_popup $ctxmmg

-proc popup_diff_menu {ctxm ctxmmg x y X Y} {
+set ctxmsm .vpane.lower.diff.body.ctxmsm
+menu $ctxmsm -tearoff 0
+$ctxmsm add command \
+	-label [mc "Visualize These Changes In The Submodule"] \
+	-command {do_gitk -- true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add command \
+	-label [mc "Visualize Current Branch History In The Submodule"] \
+	-command {do_gitk {} true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add command \
+	-label [mc "Visualize All Branch History In The Submodule"] \
+	-command {do_gitk --all true}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add separator
+$ctxmsm add command \
+	-label [mc "Start git gui In The Submodule"] \
+	-command {do_git_gui}
+lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
+$ctxmsm add separator
+create_common_diff_popup $ctxmsm
+
+proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
 	global current_diff_path file_states
 	set ::cursorX $x
 	set ::cursorY $y
@@ -3241,6 +3332,8 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
 	}
 	if {[string first {U} $state] >= 0} {
 		tk_popup $ctxmmg $X $Y
+	} elseif {$::is_submodule_diff} {
+		tk_popup $ctxmsm $X $Y
 	} else {
 		if {$::ui_index eq $::current_diff_side} {
 			set l [mc "Unstage Hunk From Commit"]
@@ -3249,7 +3342,7 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
 			set l [mc "Stage Hunk For Commit"]
 			set t [mc "Stage Line For Commit"]
 		}
-		if {$::is_3way_diff || $::is_submodule_diff
+		if {$::is_3way_diff
 			|| $current_diff_path eq {}
 			|| {__} eq $state
 			|| {_O} eq $state
@@ -3264,7 +3357,7 @@ proc popup_diff_menu {ctxm ctxmmg x y X Y} {
 		tk_popup $ctxm $X $Y
 	}
 }
-bind_button3 $ui_diff [list popup_diff_menu $ctxm $ctxmmg %x %y %X %Y]
+bind_button3 $ui_diff [list popup_diff_menu $ctxm $ctxmmg $ctxmsm %x %y %X %Y]

 # -- Status Bar
 #
-- 
1.6.6.339.g7cacc

^ permalink raw reply related

* Re: [RFC/PATCH 4/5] Documentation: reset: describe new "--keep" option
From: Daniel Convissor @ 2010-01-02 17:14 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, git, Linus Torvalds, Johannes Schindelin,
	Stephan Beyer, Daniel Barkalow, Jakub Narebski, Paolo Bonzini,
	Johannes Sixt, Stephen Boyd
In-Reply-To: <20100102053934.30066.76552.chriscool@tuxfamily.org>

On Sat, Jan 02, 2010 at 06:39:32AM +0100, Christian Couder wrote:
> +++ b/Documentation/git-reset.txt
...
> +--keep::
> +	Resets the index to match the tree recorded by the named commit,
> +	but keep changes in the working tree. Aborts if the reset would
> +	change files that are already changes in the working tree.

Grammar suggestion for the last line there.  Change "already changes" to 
"already changed".  Or better yet, use a different word, such as "already 
modified", since "change" based words have been used so many times 
already in the sentence.

Thanks,

--Dan

-- 
 T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
            data intensive web and database programming
                http://www.AnalysisAndSolutions.com/
 4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335 f: 718-854-0409

^ 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