# [PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once

10 messages from 2009-12-12 to 2009-12-12. Participants: Junio C Hamano, Jeff King, Nanako Shiraishi, Sverre Rabbelier, Jay Soffian.
Thread: https://gitlist.dev/t/21940

## Junio C Hamano, 2009-12-12 09:02

Subject: [PATCH 0/3] Update advice in commit/status output
Message-ID: <1260608523-15579-1-git-send-email-gitster@pobox.com>
URL: https://gitlist.dev/e/1260608523-15579-1-git-send-email-gitster%40pobox.com
In-Reply-To: <7vk4wtysyu.fsf@alter.siamese.dyndns.org>

```
Jay Soffian noticed that we give "git reset HEAD <path>" as an instruction
to get rid of the local change that has already been added to the index
even when <path> is unmerged, or it is merged and we are about to commit a
merge.

In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
while merging.

This miniseries updates the advices given in status/commit.  It applies on
top of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c
with the jk/unwanted-advices topic that has already graduated to 'maint'.

Junio C Hamano (3):
  commit/status: check $GIT_DIR/MERGE_HEAD only once
  commit/status: "git add <path>" is not necessarily how to resolve
  status/commit: do not suggest "reset HEAD <path>" while merging

 builtin-commit.c    |   12 ++++++------
 t/t7060-wtstatus.sh |    3 +--
 wt-status.c         |   16 +++++++++++-----
 wt-status.h         |    1 +
 4 files changed, 19 insertions(+), 13 deletions(-)

```

## Junio C Hamano, 2009-12-12 09:02

Subject: [PATCH 1/3] commit/status: check $GIT_DIR/MERGE_HEAD only once
Message-ID: <1260608523-15579-2-git-send-email-gitster@pobox.com>
URL: https://gitlist.dev/e/1260608523-15579-2-git-send-email-gitster%40pobox.com
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>

```
The code checked for the MERGE_HEAD file to see if we were about
to commit a merge twice in the codepath; also one of them used a
variable merge_head_sha1[] which was set but was never used.

Just check it once, but do so also in "git status", too, as
we will be using this for status generation in the next patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-commit.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index b39295f..17dd462 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -36,7 +36,7 @@ static const char * const builtin_status_usage[] = {
 	NULL
 };
 
-static unsigned char head_sha1[20], merge_head_sha1[20];
+static unsigned char head_sha1[20];
 static char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
 static struct lock_file index_lock; /* real index */
@@ -319,7 +319,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 	 */
 	commit_style = COMMIT_PARTIAL;
 
-	if (file_exists(git_path("MERGE_HEAD")))
+	if (in_merge)
 		die("cannot do a partial commit during a merge.");
 
 	memset(&partial, 0, sizeof(partial));
@@ -758,9 +758,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (get_sha1("HEAD", head_sha1))
 		initial_commit = 1;
 
-	if (!get_sha1("MERGE_HEAD", merge_head_sha1))
-		in_merge = 1;
-
 	/* Sanity check options */
 	if (amend && initial_commit)
 		die("You have nothing to amend.");
@@ -951,6 +948,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	wt_status_prepare(&s);
 	git_config(git_status_config, &s);
+	in_merge = file_exists(git_path("MERGE_HEAD"));
 	argc = parse_options(argc, argv, prefix,
 			     builtin_status_options,
 			     builtin_status_usage, 0);
@@ -1057,10 +1055,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	wt_status_prepare(&s);
 	git_config(git_commit_config, &s);
+	in_merge = file_exists(git_path("MERGE_HEAD"));
 
 	if (s.use_color == -1)
 		s.use_color = git_use_color_default;
-
 	argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
 					  prefix, &s);
 	if (dry_run) {
-- 
1.6.6.rc2.5.g49666

```

## Junio C Hamano, 2009-12-12 09:02

Subject: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
Message-ID: <1260608523-15579-3-git-send-email-gitster@pobox.com>
URL: https://gitlist.dev/e/1260608523-15579-3-git-send-email-gitster%40pobox.com
In-Reply-To: <1260608523-15579-2-git-send-email-gitster@pobox.com>

```
When the desired resolution is to remove the path, "git rm <path>" is the
command the user needs to use.  Just like in "Changed but not updated"
section, suggest to use "git add/rm" as appropriate.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7060-wtstatus.sh |    2 +-
 wt-status.c         |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 7b5db80..6c1af26 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -32,7 +32,7 @@ cat >expect <<EOF
 # On branch side
 # Unmerged paths:
 #   (use "git reset HEAD <file>..." to unstage)
-#   (use "git add <file>..." to mark resolution)
+#   (use "git add/rm <file>..." as appropriately to mark resolution)
 #
 #	deleted by us:      foo
 #
diff --git a/wt-status.c b/wt-status.c
index 3fdcf97..5271b6a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -52,7 +52,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
 	else
 		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
-	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to mark resolution)");
+	color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" as appropriately to mark resolution)");
 	color_fprintf_ln(s->fp, c, "#");
 }
 
-- 
1.6.6.rc2.5.g49666

```

## Junio C Hamano, 2009-12-12 09:02

Subject: [PATCH 3/3] status/commit: do not suggest "reset HEAD <path>" while merging
Message-ID: <1260608523-15579-4-git-send-email-gitster@pobox.com>
URL: https://gitlist.dev/e/1260608523-15579-4-git-send-email-gitster%40pobox.com
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>

```
Suggesting "'reset HEAD <path>' to unstage" is dead wrong if we are about
to record a merge commit.  For either an unmerged path (i.e. with
unresolved conflicts), or an updated path, it would result in discarding
what the other branch did.

Note that we do not do anything special in a case where we are amending a
merge.  The user is making an evil merge starting from an already
committed merge, and running "reset HEAD <path>" is the right way to get
rid of the local edit that has been added to the index.

Once "reset --unresolve <path>" becomes available, we might want to
suggest it for a merged path that has unresolve information, but until
then, just remove the incorrect advice.

We might also want to suggest "checkout --conflict <path>" to revert the
file in the work tree to the state of failed automerge for an unmerged
path, but we never did that, and this commit does not change that.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-commit.c    |    2 ++
 t/t7060-wtstatus.sh |    1 -
 wt-status.c         |   14 ++++++++++----
 wt-status.h         |    1 +
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 17dd462..7218454 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -960,6 +960,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	read_cache();
 	refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+	s.in_merge = in_merge;
 	wt_status_collect(&s);
 
 	if (s.relative_paths)
@@ -1056,6 +1057,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	wt_status_prepare(&s);
 	git_config(git_commit_config, &s);
 	in_merge = file_exists(git_path("MERGE_HEAD"));
+	s.in_merge = in_merge;
 
 	if (s.use_color == -1)
 		s.use_color = git_use_color_default;
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 6c1af26..6dd7077 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -31,7 +31,6 @@ test_expect_success 'Report new path with conflict' '
 cat >expect <<EOF
 # On branch side
 # Unmerged paths:
-#   (use "git reset HEAD <file>..." to unstage)
 #   (use "git add/rm <file>..." as appropriately to mark resolution)
 #
 #	deleted by us:      foo
diff --git a/wt-status.c b/wt-status.c
index 5271b6a..3f62c44 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -47,8 +47,11 @@ void wt_status_prepare(struct wt_status *s)
 static void wt_status_print_unmerged_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
+
 	color_fprintf_ln(s->fp, c, "# Unmerged paths:");
-	if (!s->is_initial)
+	if (s->in_merge)
+		;
+	else if (!s->is_initial)
 		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
 	else
 		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
@@ -59,12 +62,14 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 static void wt_status_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
+
 	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
-	if (!s->is_initial) {
+	if (s->in_merge)
+		; /* NEEDSWORK: use "git reset --unresolve"??? */
+	else if (!s->is_initial)
 		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
-	} else {
+	else
 		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
-	}
 	color_fprintf_ln(s->fp, c, "#");
 }
 
@@ -72,6 +77,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 					 int has_deleted)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
+
 	color_fprintf_ln(s->fp, c, "# Changed but not updated:");
 	if (!has_deleted)
 		color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to update what will be committed)");
diff --git a/wt-status.h b/wt-status.h
index a4bddcf..c60f40a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -34,6 +34,7 @@ struct wt_status {
 	const char **pathspec;
 	int verbose;
 	int amend;
+	int in_merge;
 	int nowarn;
 	int use_color;
 	int relative_paths;
-- 
1.6.6.rc2.5.g49666

```

## Jeff King, 2009-12-12 09:15

Subject: Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
Message-ID: <20091212091556.GA30509@coredump.intra.peff.net>
URL: https://gitlist.dev/e/20091212091556.GA30509%40coredump.intra.peff.net
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>

```
On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:

> When the desired resolution is to remove the path, "git rm <path>" is the
> command the user needs to use.  Just like in "Changed but not updated"
> section, suggest to use "git add/rm" as appropriate.

I no longer even see these messages due to advice.statushints, but the
overall direction of the series looks sane to me.

However:

> -	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to mark resolution)");
> +	color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" as appropriately to mark resolution)");

This should be "as appropriate".

-Peff

```

## Nanako Shiraishi, 2009-12-12 09:19

Subject: Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
Message-ID: <20091212181925.6117@nanako3.lavabit.com>
URL: https://gitlist.dev/e/20091212181925.6117%40nanako3.lavabit.com
In-Reply-To: <1260608523-15579-3-git-send-email-gitster@pobox.com>

```
Quoting Junio C Hamano <gitster@pobox.com>

>  # On branch side
>  # Unmerged paths:
>  #   (use "git reset HEAD <file>..." to unstage)
> -#   (use "git add <file>..." to mark resolution)
> +#   (use "git add/rm <file>..." as appropriately to mark resolution)

Shouldn't this either be "as appropriate" or just "appropriately"?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

```

## Junio C Hamano, 2009-12-12 09:24

Subject: Re: [PATCH 2/3] commit/status: "git add <path>" is not necessarily how to resolve
Message-ID: <7vbpi4v92s.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vbpi4v92s.fsf%40alter.siamese.dyndns.org
In-Reply-To: <20091212091556.GA30509@coredump.intra.peff.net>

```
Jeff King <peff@peff.net> writes:

> On Sat, Dec 12, 2009 at 01:02:02AM -0800, Junio C Hamano wrote:
>
>> When the desired resolution is to remove the path, "git rm <path>" is the
>> command the user needs to use.  Just like in "Changed but not updated"
>> section, suggest to use "git add/rm" as appropriate.
>
> I no longer even see these messages due to advice.statushints, but the
> overall direction of the series looks sane to me.
>
> However:
>
>> -	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to mark resolution)");
>> +	color_fprintf_ln(s->fp, c, "#   (use \"git add/rm <file>...\" as appropriately to mark resolution)");
>
> This should be "as appropriate".

Heh, I kant speel.  Fixed and re-pushed out on 'pu'.

Thanks.

```

## Sverre Rabbelier, 2009-12-12 16:13

Subject: Re: [PATCH 0/3] Update advice in commit/status output
Message-ID: <fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1@mail.gmail.com>
URL: https://gitlist.dev/e/fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1%40mail.gmail.com
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>

```
Heya,

On Sat, Dec 12, 2009 at 10:02, Junio C Hamano <gitster@pobox.com> wrote:
> In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
> while merging.

>From the patches I'm guessing you mean "In either case" instead?

-- 
Cheers,

Sverre Rabbelier

```

## Junio C Hamano, 2009-12-12 18:27

Subject: Re: [PATCH 0/3] Update advice in commit/status output
Message-ID: <7vws0st5e2.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vws0st5e2.fsf%40alter.siamese.dyndns.org
In-Reply-To: <fabb9a1e0912120813h1c41b7bfg4525f8f01e04ddb1@mail.gmail.com>

```
Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Sat, Dec 12, 2009 at 10:02, Junio C Hamano <gitster@pobox.com> wrote:
>> In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
>> while merging.
>
>>From the patches I'm guessing you mean "In either case" instead?

Correct.

```

## Jay Soffian, 2009-12-12 22:06

Subject: Re: [PATCH 0/3] Update advice in commit/status output
Message-ID: <76718490912121406q31ddcd12s3d813659d5096a22@mail.gmail.com>
URL: https://gitlist.dev/e/76718490912121406q31ddcd12s3d813659d5096a22%40mail.gmail.com
In-Reply-To: <1260608523-15579-1-git-send-email-gitster@pobox.com>

```
On Sat, Dec 12, 2009 at 4:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian noticed that we give "git reset HEAD <path>" as an instruction
> to get rid of the local change that has already been added to the index
> even when <path> is unmerged, or it is merged and we are about to commit a
> merge.
>
> In neither case, "git reset HEAD <path>" is absolutely a wrong thing to do
> while merging.
>
> This miniseries updates the advices given in status/commit.  It applies on
> top of the jk/1.7.0-status topic, and has trivial conflicts in wt-status.c
> with the jk/unwanted-advices topic that has already graduated to 'maint'.

Series looks good to me after the spelling correction. Thank you.

j.

```