Git development
 help / color / mirror / Atom feed
* [PATCH v2 3/4] status: show more info than "currently not on any branch"
From: Nguyễn Thái Ngọc Duy @ 2013-03-06 12:21 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Matthieu Moy, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1362572482-20570-1-git-send-email-pclouds@gmail.com>

When a remote ref or a tag is checked out, HEAD is automatically
detached. There is no user-friendly way to find out what ref is
checked out in this case. This patch digs in reflog for this
information and shows "HEAD detached from origin/master" or "HEAD
detached at v1.8.0" instead of "currently not on any branch".

When it cannot figure out the original ref, it shows an abbreviated
SHA-1. "Currently not on any branch" would never display (unless
reflog is pruned to near empty that the last checkout entry is lost).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t7406-submodule-update.sh |  6 ++--
 t/t7512-status-help.sh      | 52 +++++++++++++++++----------
 wt-status.c                 | 85 ++++++++++++++++++++++++++++++++++++++++++---
 wt-status.h                 |  4 ++-
 4 files changed, 121 insertions(+), 26 deletions(-)

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 4975ec0..50ac020 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -664,8 +664,10 @@ test_expect_success 'submodule add properly re-creates deeper level submodules'
 
 test_expect_success 'submodule update properly revives a moved submodule' '
 	(cd super &&
+	 H=$(git rev-parse --short HEAD) &&
 	 git commit -am "pre move" &&
-	 git status >expect&&
+	 H2=$(git rev-parse --short HEAD) &&
+	 git status | sed "s/$H/XXX/" >expect &&
 	 H=$(cd submodule2; git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
@@ -674,7 +676,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git config -f .gitmodules submodule.submodule2.path "moved/sub module"
 	 git commit -am "post move" &&
 	 git submodule update &&
-	 git status >actual &&
+	 git status | sed "s/$H2/XXX/" >actual &&
 	 test_cmp expect actual
 	)
 '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index d2da89a..da22088 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -76,7 +76,7 @@ test_expect_success 'status when rebase in progress before resolving conflicts'
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -103,7 +103,7 @@ test_expect_success 'status when rebase in progress before rebase --continue' '
 	echo three >main.txt &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_conflicts'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -135,7 +135,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' '
 	ONTO=$(git rev-parse --short rebase_i_conflicts) &&
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (fix conflicts and then run "git rebase --continue")
 	#   (use "git rebase --skip" to skip this patch)
@@ -161,7 +161,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' '
 	test_must_fail git rebase -i rebase_i_conflicts &&
 	git add main.txt &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''.
 	#   (all conflicts fixed: run "git rebase --continue")
 	#
@@ -187,9 +187,10 @@ test_expect_success 'status when rebasing -i in edit mode' '
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~2) &&
+	TGT=$(git rev-parse --short two_rebase_i) &&
 	git rebase -i HEAD~2 &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -214,8 +215,9 @@ test_expect_success 'status when splitting a commit' '
 	ONTO=$(git rev-parse --short HEAD~3) &&
 	git rebase -i HEAD~3 &&
 	git reset HEAD^ &&
+	TGT=$(git rev-parse --short HEAD) &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $TGT
 	# You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -243,10 +245,11 @@ test_expect_success 'status after editing the last commit with --amend during a
 	export FAKE_LINES &&
 	test_when_finished "git rebase --abort" &&
 	ONTO=$(git rev-parse --short HEAD~3) &&
+	TGT=$(git rev-parse --short three_amend) &&
 	git rebase -i HEAD~3 &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $TGT
 	# You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -276,7 +279,7 @@ test_expect_success 'status: (continue first edit) second edit' '
 	git rebase -i HEAD~3 &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -298,7 +301,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -325,7 +328,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "foo" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -347,7 +350,7 @@ test_expect_success 'status: (amend first edit) second edit' '
 	git commit --amend -m "a" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -370,7 +373,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -398,7 +401,7 @@ test_expect_success 'status: (amend first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "d" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -422,7 +425,7 @@ test_expect_success 'status: (split first edit) second edit' '
 	git commit -m "e" &&
 	git rebase --continue &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -447,7 +450,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
 	git rebase --continue &&
 	git reset HEAD^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (Once your working directory is clean, run "git rebase --continue")
 	#
@@ -477,7 +480,7 @@ test_expect_success 'status: (split first edit) second edit and amend' '
 	git rebase --continue &&
 	git commit --amend -m "h" &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached from $ONTO
 	# You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''.
 	#   (use "git commit --amend" to amend the current commit)
 	#   (use "git rebase --continue" once you are satisfied with your changes)
@@ -572,8 +575,9 @@ test_expect_success 'status when bisecting' '
 	git bisect start &&
 	git bisect bad &&
 	git bisect good one_bisect &&
-	cat >expected <<-\EOF &&
-	# Not currently on any branch.
+	TGT=$(git rev-parse --short two_bisect) &&
+	cat >expected <<-EOF &&
+	# HEAD detached at $TGT
 	# You are currently bisecting branch '\''bisect'\''.
 	#   (use "git bisect reset" to get back to the original branch)
 	#
@@ -596,7 +600,7 @@ test_expect_success 'status when rebase conflicts with statushints disabled' '
 	ONTO=$(git rev-parse --short HEAD^^) &&
 	test_must_fail git rebase HEAD^ --onto HEAD^^ &&
 	cat >expected <<-EOF &&
-	# Not currently on any branch.
+	# HEAD detached at $ONTO
 	# You are currently rebasing branch '\''statushints_disabled'\'' on '\''$ONTO'\''.
 	#
 	# Unmerged paths:
@@ -662,5 +666,15 @@ test_expect_success 'status when cherry-picking after resolving conflicts' '
 	test_i18ncmp expected actual
 '
 
+test_expect_success 'status showing detached from a tag' '
+	test_commit atag tagging &&
+	git checkout atag &&
+	cat >expected <<-\EOF
+	# HEAD detached at atag
+	nothing to commit (use -u to show untracked files)
+	EOF
+	git status --untracked-files=no >actual &&
+	test_i18ncmp expected actual
+'
 
 test_done
diff --git a/wt-status.c b/wt-status.c
index 6a3566b..7797ccf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1001,7 +1001,68 @@ static void read_and_strip_branch(struct strbuf *sb,
 		*branch = xstrdup(*branch);
 }
 
-void wt_status_get_state(struct wt_status_state *state)
+struct grab_1st_switch_cbdata {
+	struct strbuf buf;
+	unsigned char nsha1[20];
+};
+
+static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+			   const char *email, unsigned long timestamp, int tz,
+			   const char *message, void *cb_data)
+{
+	struct grab_1st_switch_cbdata *cb = cb_data;
+	const char *target = NULL, *end;
+
+	if (prefixcmp(message, "checkout: moving from "))
+		return 0;
+	message += strlen("checkout: moving from ");
+	target = strstr(message, " to ");
+	if (!target)
+		return 0;
+	target += strlen(" to ");
+	strbuf_reset(&cb->buf);
+	hashcpy(cb->nsha1, nsha1);
+	for (end = target; *end && *end != '\n'; end++)
+		;
+	strbuf_add(&cb->buf, target, end - target);
+	return 0;
+}
+
+static void wt_status_get_detached_from(struct wt_status_state *state)
+{
+	struct grab_1st_switch_cbdata cb;
+	struct commit *commit;
+	unsigned char sha1[20];
+	char *ref = NULL;
+
+	strbuf_init(&cb.buf, 0);
+	if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
+		for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
+	if (!cb.buf.len)
+		return;
+
+	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
+	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
+	    !hashcmp(cb.nsha1, commit->object.sha1)) {
+		int ofs;
+		if (!prefixcmp(ref, "refs/tags/"))
+			ofs = strlen("refs/tags/");
+		else if (!prefixcmp(ref, "refs/remotes/"))
+			ofs = strlen("refs/remotes/");
+		else
+			ofs = 0;
+		state->detached_from = xstrdup(ref + ofs);
+	} else
+		state->detached_from =
+			xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
+	hashcpy(state->detached_sha1, cb.nsha1);
+
+	free(ref);
+	strbuf_release(&cb.buf);
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+			 int get_detached_from)
 {
 	struct strbuf branch = STRBUF_INIT;
 	struct strbuf onto = STRBUF_INIT;
@@ -1040,6 +1101,10 @@ void wt_status_get_state(struct wt_status_state *state)
 		read_and_strip_branch(&branch, &state->branch,
 				      "BISECT_START");
 	}
+
+	if (get_detached_from)
+		wt_status_get_detached_from(state);
+
 	strbuf_release(&branch);
 	strbuf_release(&onto);
 }
@@ -1066,7 +1131,8 @@ void wt_status_print(struct wt_status *s)
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
 	struct wt_status_state state;
 
-	wt_status_get_state(&state);
+	wt_status_get_state(&state,
+			    s->branch && !strcmp(s->branch, "HEAD"));
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1074,9 +1140,19 @@ void wt_status_print(struct wt_status *s)
 		if (!prefixcmp(branch_name, "refs/heads/"))
 			branch_name += 11;
 		else if (!strcmp(branch_name, "HEAD")) {
-			branch_name = "";
 			branch_status_color = color(WT_STATUS_NOBRANCH, s);
-			on_what = _("Not currently on any branch.");
+			if (state.detached_from) {
+				unsigned char sha1[20];
+				branch_name = state.detached_from;
+				if (!get_sha1("HEAD", sha1) &&
+				    !hashcmp(sha1, state.detached_sha1))
+					on_what = _("HEAD detached at ");
+				else
+					on_what = _("HEAD detached from ");
+			} else {
+				branch_name = "";
+				on_what = _("Not currently on any branch.");
+			}
 		}
 		status_printf(s, color(WT_STATUS_HEADER, s), "");
 		status_printf_more(s, branch_status_color, "%s", on_what);
@@ -1088,6 +1164,7 @@ void wt_status_print(struct wt_status *s)
 	wt_status_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
+	free(state.detached_from);
 
 	if (s->is_initial) {
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
diff --git a/wt-status.h b/wt-status.h
index 5ddcbf6..5cb7df9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -81,12 +81,14 @@ struct wt_status_state {
 	int bisect_in_progress;
 	char *branch;
 	char *onto;
+	char *detached_from;
+	unsigned char detached_sha1[20];
 };
 
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state);
+void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply related

* [PATCH v2 4/4] branch: show more information when HEAD is detached
From: Nguyễn Thái Ngọc Duy @ 2013-03-06 12:21 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Matthieu Moy, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1362572482-20570-1-git-send-email-pclouds@gmail.com>

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c            | 25 ++++++++++++++++++++++++-
 t/t3203-branch-output.sh    |  6 +++---
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..02dee0d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description()
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +580,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 76fe7e0..ba4f98e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -94,13 +94,13 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
-cat >expect <<'EOF'
-* (no branch)
+test_expect_success 'git branch shows detached HEAD properly' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
 EOF
-test_expect_success 'git branch shows detached HEAD properly' '
 	git checkout HEAD^0 &&
 	git branch >actual &&
 	test_i18ncmp expect actual
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply related

* [PATCH v2 0/4] nd/branch-show-rebase-bisect-state updates
From: Nguyễn Thái Ngọc Duy @ 2013-03-06 12:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Matthieu Moy, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1362572482-20570-5-git-send-email-pclouds@gmail.com>

Since the previous round:

 - reflog format change is dropped
 - almost always show "HEAD detached from/at xxx" instead of
   "currently not on any branch", where "xxx" is either a ref, or an
   abbreviated sha-1

I wanted to introduce something like @{-1:to} that resolves the "to"
sha-1 in reflog, in addition to @{-1} which resolves the "from" part.
But the syntax looks ugly so I dropped it. Meanwhile people could do

git log -1 -g --grep-reflog checkout --pretty=format:%h

to retrieve the same info.

Nguyễn Thái Ngọc Duy (4):
  wt-status: split wt_status_state parsing function out
  wt-status: move wt_status_get_state() out to wt_status_print()
  status: show more info than "currently not on any branch"
  branch: show more information when HEAD is detached

 builtin/branch.c            |  25 +++++++-
 t/t3203-branch-output.sh    |   6 +-
 t/t6030-bisect-porcelain.sh |   2 +-
 t/t7406-submodule-update.sh |   6 +-
 t/t7512-status-help.sh      |  52 +++++++++------
 wt-status.c                 | 152 ++++++++++++++++++++++++++++++++++----------
 wt-status.h                 |   7 +-
 7 files changed, 190 insertions(+), 60 deletions(-)

-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply

* [PATCH v2+ 4/4] branch: show more information when HEAD is detached
From: Nguyễn Thái Ngọc Duy @ 2013-03-06 12:26 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Matthieu Moy, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1362572773-20809-1-git-send-email-pclouds@gmail.com>

This prints more helpful info when HEAD is detached: is it detached
because of bisect or rebase? What is the original branch name in those
cases? Is it detached because the user checks out a remote ref or a
tag (and which one)?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Oops, the previous one did not do

  s/\(get_head_description\)()$/\1(void)/
 
 This one does.

 builtin/branch.c            | 25 ++++++++++++++++++++++++-
 t/t3203-branch-output.sh    |  6 +++---
 t/t6030-bisect-porcelain.sh |  2 +-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6371bf9..02dee0d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -18,6 +18,7 @@
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
+#include "wt-status.h"
 
 static const char * const builtin_branch_usage[] = {
 	N_("git branch [options] [-r | -a] [--merged | --no-merged]"),
@@ -550,6 +551,28 @@ static int calc_maxwidth(struct ref_list *refs)
 	return w;
 }
 
+static char *get_head_description(void)
+{
+	struct strbuf desc = STRBUF_INIT;
+	struct wt_status_state state;
+	wt_status_get_state(&state, 1);
+	if (state.rebase_in_progress ||
+	    state.rebase_interactive_in_progress)
+		strbuf_addf(&desc, _("(no branch, rebasing %s)"),
+			    state.branch);
+	else if (state.bisect_in_progress)
+		strbuf_addf(&desc, _("(no branch, bisecting %s)"),
+			    state.branch);
+	else if (state.detached_from)
+		strbuf_addf(&desc, _("(detached from %s)"),
+			    state.detached_from);
+	else
+		strbuf_addstr(&desc, _("(no branch)"));
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+	return strbuf_detach(&desc, NULL);
+}
 
 static void show_detached(struct ref_list *ref_list)
 {
@@ -557,7 +580,7 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup(_("(no branch)"));
+		item.name = get_head_description();
 		item.width = utf8_strwidth(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 76fe7e0..ba4f98e 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -94,13 +94,13 @@ test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
 '
 
-cat >expect <<'EOF'
-* (no branch)
+test_expect_success 'git branch shows detached HEAD properly' '
+	cat >expect <<EOF &&
+* (detached from $(git rev-parse --short HEAD^0))
   branch-one
   branch-two
   master
 EOF
-test_expect_success 'git branch shows detached HEAD properly' '
 	git checkout HEAD^0 &&
 	git branch >actual &&
 	test_i18ncmp expect actual
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 3e0e15f..9b6f0d0 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if
 	cp .git/BISECT_START saved &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
-	test_i18ngrep "* (no branch)" branch.output > /dev/null &&
+	test_i18ngrep "* (no branch, bisecting other)" branch.output > /dev/null &&
 	test_cmp saved .git/BISECT_START
 '
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply related

* [PATCH] git-push.txt: mention about remote.*.push when no refspec is given
From: Nguyễn Thái Ngọc Duy @ 2013-03-06 12:44 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

remote.*.push is considered before push.default, but there's no
mention about that, except a bit in the examples. The description
of push.default does say something about this, but it would be easier
to find out if it's described here.

The first paragraph describing refspec format is moved down, so that
the new first paragraph is about unspsecified refspec and the
remaining paragraphs refspec format.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-push.txt | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 1398025..20bf2f5 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -33,13 +33,16 @@ OPTIONS[[OPTIONS]]
 	of a remote (see the section <<REMOTES,REMOTES>> below).
 
 <refspec>...::
-	The format of a <refspec> parameter is an optional plus
-	`+`, followed by the source ref <src>, followed
-	by a colon `:`, followed by the destination ref <dst>.
-	It is used to specify with what <src> object the <dst> ref
-	in the remote repository is to be updated.  If not specified,
-	the behavior of the command is controlled by the `push.default`
-	configuration variable.
+	Specify how how refs are pushed. If not specified and
+	configuration variable `remote.<remote>.push` is set, the
+	variable is used as default refspec. If not specified and the
+	variable is not set, the behavior of the command is controlled
+	by the `push.default` configuration variable.
++
+The format of a <refspec> parameter is an optional plus `+`, followed
+by the source ref <src>, followed by a colon `:`, followed by the
+destination ref <dst>.  It is used to specify with what <src> object
+the <dst> ref in the remote repository is to be updated.
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
-- 
1.8.1.2.536.gf441e6d

^ permalink raw reply related

* Re: [PATCH] help: show manpage for aliased command on git <alias> --help
From: Michael J Gruber @ 2013-03-06 13:10 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
	Jonathan Nieder, H.Merijn Brand
In-Reply-To: <20130305173831.GB9379@sigill.intra.peff.net>

Jeff King venit, vidit, dixit 05.03.2013 18:38:
> On Tue, Mar 05, 2013 at 02:44:41PM +0000, Ævar Arnfjörð Bjarmason wrote:
> 
>> Change the semantics of "git <alias> --help" to show the help for the
>> command <alias> is aliased to, instead of just saying:
>>
>>     `git <alias>' is aliased to `<whatever>'
>>
>> E.g. if you have "checkout" aliased to "co" you won't get:
>>
>>     $ git co --help
>>     `git co' is aliased to `checkout'
>>
>> But will instead get the manpage for git-checkout. The behavior this
>> is replacing was originally added by Jeff King in 2156435. I'm
>> changing it because of this off-the-cuff comment on IRC:
>>
>>     14:27:43 <@Tux> git can be very unhelpful, literally:
>>     14:27:46 <@Tux> $ git co --help
>>     14:27:46 <@Tux> `git co' is aliased to `checkout'
>>     14:28:08 <@Tux> I know!, gimme the help for checkout, please
>>
>> And because I also think it makes more sense than showing you what the
>> thing is aliased to.
> 
> In this simple case, I think it is helpful to show the "checkout"
> manpage, because there is no other information to give (and by showing
> the checkout manpage, you implicitly indicate that "co" maps to
> "checkout").

Well, even in the simple case one has to wonder: Why does the user
invoke help for "co"? There are two very likely cases:

A) User does not remember what "co" is aliased to.
B) User wants to see the man page.

If A is not the case then it's easy for the user to request help for
"checkout" (or "commit" or ...).

Removing the only easy way to look up the definition of an alias is a
major regression. And no, "git config --get alias.co" is not an easy way.

Removing that easy way would be OK only after introducing a "git alias"
command which works similar to shell's alias. You can even set it up as
an alias for git config, of course.

> But like others, I am concerned about the other cases, where there is no
> manpage, it is not a git command with a manpage, or it is a git command
> with options.  You are losing useful information that is currently given
> to the user in all but the single-word case.
> 
> In an ideal world, we could say "here is how the alias expands, and by
> the way, here is the manpage for the expanded command". And obviously
> just omit the latter part when there is no such page. But we are relying
> on external programs to do the presentation and paging. Doing the
> C equivalent of:
> 
>   echo "'git co' is aliased to 'checkout'" &&
>   man checkout
> 
> does not quite work, because "man" will start a pager. We can run our
> own pager (which should suppress man's invocation), but that is a
> regression for anyone who uses MANPAGER.
> 
> The user may also be using help.format to use something besides man. If
> help.format is set to "html", we will spawn a browser. In that case we
> can still output the alias information, but it may or may not be seen
> (though come to think of it, that is probably already a problem for "git
> help <alias>" on Windows systems, or anybody invoking git help from a
> GUI porcelain).
> 
> So I'd only be in favor of this patch if it managed to avoid information
> loss in the more complicated cases. And I'm not sure how best to do
> that. The "only trigger for a single-word alias" suggestion seems like
> the least ugly to me.
> 
> -Peff
> 

^ permalink raw reply

* Re: feature suggestion: improve rerere
From: Johannes Sixt @ 2013-03-06 13:24 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: git, kernel
In-Reply-To: <20130306101640.GA15375@pengutronix.de>

Am 3/6/2013 11:16, schrieb Uwe Kleine-König:
> 	++<<<<<<< ours
> 	 +ssize_t xread(int fd, void *buf, size_t count)
> 	 +{
> 	 +	ssize_t ret, done = 0;
> 	 +
> 	 +retry:
> 	 +	ret = read(fd, buf + done, count - done);
> 	 +	if (ret < 0)
> 	 +		return ret;
> 	 +
> 	 +	done += ret;
> 	 +
> 	 +	if (ret == 0 /* EOF */ || done == count)
> 	 +		return done;
> 	 +	else
> 	 +		goto retry;
> 	 +}
> 	 +
> 	++||||||| base
> 	++=======
> 	+ #include "common.h"
> 	+ 
> 	++>>>>>>> theirs
> 	  int main(int argc,char *argv[])
> 	  {
> 		int fd, val, ret, size, wrote, len;
> 
> This is the same conflict as the first one, just with ours and theirs
> exchanged. So my suggestion is to make rerere use the resolution
> recorded for the first conflict here.
> 
> Sounds sensible?

Of course, and rerere already does it. But only when you use git's default
conflict markers rather than diff3 style markers that have this extra
||||| line.

-- Hannes

^ permalink raw reply

* Splitting config.txt
From: Duy Nguyen @ 2013-03-06 14:01 UTC (permalink / raw)
  To: git

We discussed this before about adding configuration variables into
individual command man pages [1]. This may be a step towards
that. With this, I could add selected configuration variables to
git-push.txt, for example, with a simple patch:

-- 8< --
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 1398025..c130c90 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -348,6 +348,13 @@ you are certain that nobody in the meantime fetched your earlier commit A
 overwrite it. In other words, "git push --force" is a method reserved for
 a case where you do mean to lose history.
 
+CONFIGURATION
+-------------
+
+include::config/push.default.txt[]
+include::config/remote.name.pushurl.txt[]
+include::config/remote.name.push.txt[]
+
 
 Examples
 --------
-- 8< --

I know it makes man pages longer (and more intimidating for new
users), but the point is we could select just important keys for each
man page, instead of all keys affecting the command.

I don't post the final patch that splits config.txt, just the script I
use to split it. You can try it yourself. It puts each config key into
a file under Documentation/config, with some characters mangled to be
more fs friendly. Run it inside Documentation/. generated git-config.1
is the same after the split.

-- 8< --
#!/usr/bin/perl

sub extract {
    my ($line) = @_;
    open CC, ">config/$filename" || die "failed to open config/$filename";
    foreach $l (@lines[($start - 1)..($line - 2)]) {
	$l =~ s/^include::/include::..\//;
	print CC $l;
    }
    close CC;
    print C "include::config/$filename" . "[]\n";
}

open C, "config.txt" || die "failed to open config.txt";
our @lines = <C>;
close C;

our $start = 0;
our $filename;

open F, "grep -n '^[a-z].*::\$' config.txt|" || die "failed to grep";
open C, ">config.txt.new" || die "unable to open config.txt.new";
while (<F>) {
    chomp;
    $_ =~ m/([^:]*):(.*)/;
    my $line = $1;
    my $name = $2;
    $name =~ s/\*/_/g;
    $name =~ s/<//g;
    $name =~ s/>//g;
    $name =~ s/::$//;
    $name = "http.speedLimit" if $name eq "http.lowSpeedLimit, http.lowSpeedTime";
    $name = "gitcvs.userpass" if $name eq "gitcvs.dbuser, gitcvs.dbpass";
    next if $line - $start == 1;
    if ($start > 0) {
	extract $line;
    } else {
	foreach $l (@lines[0..($line-2)]) {
	    print C $l;
	}
    }
    $start = $line;
    $filename = $name . ".txt";
}

extract $#lines + 2;
close C;
system "mv config.txt.new config.txt";
-- 8< --

[1] http://thread.gmane.org/gmane.comp.version-control.git/206780/focus=206939

^ permalink raw reply related

* Re: feature suggestion: improve rerere
From: Uwe Kleine-König @ 2013-03-06 14:42 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, kernel
In-Reply-To: <51374382.6050101@viscovery.net>

On Wed, Mar 06, 2013 at 02:24:18PM +0100, Johannes Sixt wrote:
> Am 3/6/2013 11:16, schrieb Uwe Kleine-König:
> > 	++<<<<<<< ours
> > 	 +ssize_t xread(int fd, void *buf, size_t count)
> > 	 +{
> > 	 +	ssize_t ret, done = 0;
> > 	 +
> > 	 +retry:
> > 	 +	ret = read(fd, buf + done, count - done);
> > 	 +	if (ret < 0)
> > 	 +		return ret;
> > 	 +
> > 	 +	done += ret;
> > 	 +
> > 	 +	if (ret == 0 /* EOF */ || done == count)
> > 	 +		return done;
> > 	 +	else
> > 	 +		goto retry;
> > 	 +}
> > 	 +
> > 	++||||||| base
> > 	++=======
> > 	+ #include "common.h"
> > 	+ 
> > 	++>>>>>>> theirs
> > 	  int main(int argc,char *argv[])
> > 	  {
> > 		int fd, val, ret, size, wrote, len;
> > 
> > This is the same conflict as the first one, just with ours and theirs
> > exchanged. So my suggestion is to make rerere use the resolution
> > recorded for the first conflict here.
> > 
> > Sounds sensible?
> 
> Of course, and rerere already does it. But only when you use git's default
> conflict markers rather than diff3 style markers that have this extra
> ||||| line.
I only did git checkout --conflict=diff3 after the merge conflict
happend. So I cannot confirm that git already does it.

So here is a reproduction receipe:

	git clone git://git.infradead.org/mtd-utils.git
	cd mtd-utils
	git checkout ca39eb1
	wget -O patch1 http://article.gmane.org/gmane.linux.drivers.mtd/45779/raw
	wget -O patch2 http://article.gmane.org/gmane.linux.drivers.mtd/45591/raw
	for p in patch1 patch2; do perl -p -i -e 'print "From tralala Mon Sep 17 00:00:00 2001\n" if $. == 1' $p; done
	git am patch1
	git am -3 patch2 # first merge conflict
	perl -n -i -e 's/=======//; print unless /^[<>]{7} /;' flash_otp_write.c # resolve
	git add flash_otp_write.c
	git am --resolved
	git rebase -i ca39eb1 # swap order of the two patches

results in

	$ git ls-files -u
	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 1	flash_otp_write.c
	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 2	flash_otp_write.c
	100644 31b963e2d6cf0016ca542529886e1ee71a22664e 3	flash_otp_write.c

and resolving yields:

	$ git ls-files -s flash_otp_write.c
	100644 648e0422d21c0ffa7621f82b86c02a065e488293 0	flash_otp_write.c

Then
	git rebase --continue 

gives the 2nd rebase conflict:

	$ git ls-files -u
	100644 d407ebbf400e630dc00ee004ecb44be8af51b25d 1	flash_otp_write.c
	100644 648e0422d21c0ffa7621f82b86c02a065e488293 2	flash_otp_write.c
	100644 f360a3e025deaf7acfb7b20c9fad90f498ae4430 3	flash_otp_write.c

Now knowing from the previous resolution that with base=f360a3e0
(= origin + patch1) merging

	d407ebbf (= origin) and
	31b963e2 (= origin + patch1 + patch2)

gives 648e0422 (origin + patch2),
git could know that with base=d407ebbf (origin) merging 648e0422 (origin
+ patch1) and f360a3e0 (origin + patch1) gives 31b963e2 (origin + patch1
+ patch2) again.

And git doesn't prepare 31b963e2 in flash_otp_write.c for me.

@Johannes, do you have some non-standard setting, or can you reproduce?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* feature suggestion: optimize common parts for checkout --conflict=diff3
From: Uwe Kleine-König @ 2013-03-06 15:05 UTC (permalink / raw)
  To: git; +Cc: kernel

Hello,

Here comes another recipe for a different suggestion:

	git init
	echo 1 > file
	git add file
	git commit -m 'base'
	git branch branch
	seq 1 30 | grep -v 15 > file
	git commit -m 'add 2-30 without 15' file
	git checkout branch
	seq 1 30 | grep -v 16 > file
	git commit -m 'add 2-30 without 16' file
	git merge master
	git diff

This yields:

	diff --cc file
	index a07e697,5080129..0000000
	--- a/file
	+++ b/file
	@@@ -12,7 -12,7 +12,11 @@@
	  12
	  13
	  14
	++<<<<<<< HEAD
	 +15
	++=======
	+ 16
	++>>>>>>> master
	  17
	  18
	  19

as expected; nice and sweet. After

	git checkout --conflict=diff3 file

however the difference isn't that easy to spot any more. I expected

	diff --cc file
	index a07e697,5080129..0000000
	--- a/file
	+++ b/file
	@@@ -12,7 -12,7 +12,12 @@@
	  12
	  13
	  14
	++<<<<<<< ours
	 +15
	++||||||| base
	++=======
	+ 16
	++>>>>>>> theirs
	  17
	  18
	  19

But instead I get

	diff --cc file
	index a07e697,5080129..0000000
	--- a/file
	+++ b/file
	@@@ -1,29 -1,29 +1,61 @@@
	  1
	++<<<<<<< ours
	 +2
	 +3
	 +4
	 +5
	 +6
	 +7
	 +8
	 +9
	 +10
	 +11
	 +12
	 +13
	 +14
	 +15
	 +17
	 +18
	 +19
	 +20
	 +21
	 +22
	 +23
	 +24
	 +25
	 +26
	 +27
	 +28
	 +29
	 +30
	++||||||| base
	++=======
	+ 2
	+ 3
	+ 4
	+ 5
	+ 6
	+ 7
	+ 8
	+ 9
	+ 10
	+ 11
	+ 12
	+ 13
	+ 14
	+ 16
	+ 17
	+ 18
	+ 19
	+ 20
	+ 21
	+ 22
	+ 23
	+ 24
	+ 25
	+ 26
	+ 27
	+ 28
	+ 29
	+ 30
	++>>>>>>> theirs

Of course this is technically correct, just not maximally helpful.

Is this a missing optimisation for the diff3 case or did I miss a detail
that makes my expectation wrong?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH v2 1/4] commit.c: add clear_commit_marks_many()
From: Junio C Hamano @ 2013-03-06 15:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20130306083037.GA2018@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Mar 05, 2013 at 02:47:16PM -0800, Junio C Hamano wrote:
>
>> clear_commit_marks(struct commit *, unsigned) only can clear flag
>> bits starting from a single commit; introduce an API to allow
>> feeding an array of commits, so that flag bits can be cleared from
>> commits reachable from any of them with a single traversal.
>
> Out of curiosity, is that actually measurably more efficient?
>
> Since we stop traversing a commit's parents when we see it does not have
> any of the flags we are clearing, we should visit most commits once. The
> exception is ones that we hit by multiple paths. But that should
> be the same whether it is done as part of a single traversal or
> multiple; in each case, we hit the commit and say "Oh, already cleared;
> do not add it to the parents list".
>
> So I would expect it to have little to no impact on performance.  I
> still think it is a sane interface change; I was just curious from the
> commit message whether there could be a performance impact.

I agree that the log message should end with "with a single API
call." to clarify that this is more about eliminating the code
overhead (the caller having to loop over an array of things) than
about runtime overhead.

^ permalink raw reply

* Re: [PATCH v2 0/4] push --follow-tags
From: Junio C Hamano @ 2013-03-06 15:55 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20130306084105.GB2018@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Mar 05, 2013 at 02:47:15PM -0800, Junio C Hamano wrote:
>
>> The primary change since the last round is that it pushes out only
>> annotated tags that are missing from the other side.
>
> Like you, I have mixed feelings on treating annotated tags separately. I
> don't feel like the previous discussion actually came to a conclusion.
>
> I kind of lean towards the "if we are pushing A..B to a ref,...

I rejected that approach when I started writing the initial round of
this topic, primarily because A (i.e. dst_ref->old_sha1) may not
necessarily be what the current pusher pushed out during his last
push.

The criteria for the tip of a branch to be good enough to be pushed
out and the criteria for a commit to be good enough to be tagged are
different.  You may know that a commit together with the history
behind it is solid and may be sure that you will not have to rewind
beyond that commit---that is when you _can_ push out (you do not
have to, and I usually don't, so this is not a problem for me
personally).

But at that point in time, you may still not know if the commit and
the history behind it is the good point to be tagged for a release,
or you would still need some other commits on top to polish other
areas for the tagged release is complete.

As I made a habit of not pushing anything out if I am planning to
tag in a near future (and my tagged commits are ones designed to be
tagged, not just an ordinary commit that happens to be tagged), it
is not a problem for me, but it would be for "centralized workflow"
people, where there is more pressure to publish a commit as early as
possible.  Once you know the commit is good enough so that you do
not anticipate that it will need to be rewound, you want to push it
out so that you can unblock others who want to build on it.  It is
plausible to see:

    (1) The pumpking pushes 'master' after being satisfied the
        correctness of the history that leads to it.  The pumpking
        hasn't decided if the commit is to be tagged, or needs some
        other commits on top, for the next release.  But he prefers
        to push the current state out so that he can unblock other
        people.

    (2) Another committer pulls and pushes with his changes.

    (3) The pumpking tags what he pushed out with (1), attempts a
        push, fails the ff check, pulls (2) and then pushes the
        result out with "push --follow-tags".

^ permalink raw reply

* Re: [PATCH] add: Clarify documentation of -A and -u
From: Junio C Hamano @ 2013-03-06 17:10 UTC (permalink / raw)
  To: Greg Price; +Cc: git
In-Reply-To: <20130306072610.GB15534@biohazard-cafe.mit.edu>

Greg Price <price@MIT.EDU> writes:

> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 388a225..f89d920 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -105,7 +105,9 @@ apply to the index. See EDITING PATCHES below.
>  	will never stage new files, but that it will stage modified
>  	new contents of tracked files and that it will remove files
>  	from the index if the corresponding files in the working tree
> -	have been removed.
> +	have been removed.  By contrast `-A` will add new files as
> +	well as updating and removing existing ones, and the default
> +	behavior will add new files and will not remove existing ones.
>  +
>  If no <pathspec> is given, the current version of Git defaults to
>  "."; in other words, update all tracked files in the current directory

I do not know if mentioning what -A does in the description for -u
(and vice versa) makes it easier to understand or more confusing
(not rhetorical: I suspect some may find it easier and others not).

But "and the default behaviour will..." here _is_ confusing.  After
reading this patch three times, I still cannot tell what "default"
you are trying to describe.  Is it "-u" without arguments?  Is it
"add" without "-u" nor "-A"?  Is it something else???

Exactly the same comment applies to the other hunk.

Having said that, I agree that the current description for "-u" is
way suboptimal.  It begins like this:

    -u::
    --update::
            Only match <pathspec> against already tracked files in
            the index rather than the working tree. That means that it
            will ...

Whenever you see an incomprehensible technobabble followed by "That
means", "In other words", etc., you (not limited to Greg-you but
figuratively "everybody") should see if it makes it easier to read
to remove everything up to that "That means" and adding a bit more
to the remainder.  In this particular case, the technobabble is not
even correct---"match"ing is only a small part of what "add -u" does
to find what to "add".

I would suggest rewriting the first part of "-u" perhaps like this:

	-u::
        --update::
		Update modified and/or removed paths in the index
		that match <pathspec> with the current state of the
		working tree files.  No new path is added because
		this considers only the paths that are already in
		the index.


The text for "-A" may look like this:

	-A::
        --all::
		Update the index to record the current state of the
		working tree files that match <pathspec>.  Note that
		new paths will be added to the index, in addition to
		modified and/or removed paths.

I agree with your patch that "If no <pathspec> is given" should be
repeated for both "-u" and "-A".

> @@ -114,10 +116,17 @@ of Git, hence the form without <pathspec> should not be used.
>  
>  -A::
>  --all::
> -	Like `-u`, but match <pathspec> against files in the
> -	working tree in addition to the index. That means that it
> -	will find new files as well as staging modified content and
> -	removing files that are no longer in the working tree.
> +	Update the index regarding all files that match <pathspec>,
> +	either in the index or the working tree.  That is, remove
> +	files that are only in the index, add files that are only in
> +	the working tree, and update files that differ between the
> +	two.  By contrast `-u` only removes and updates, and the
> +	default behavior only adds and updates.
> ++
> +If no <pathspec> is given, the current version of Git defaults to
> +"."; in other words, update all tracked files in the current directory
> +and its subdirectories. This default will change in a future version
> +of Git, hence the form without <pathspec> should not be used.
>  
>  -N::
>  --intent-to-add::

^ permalink raw reply

* Re: [PATCH] git-push.txt: mention about remote.*.push when no refspec is given
From: Junio C Hamano @ 2013-03-06 18:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1362573848-21793-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> remote.*.push is considered before push.default, but there's no
> mention about that, except a bit in the examples. The description
> of push.default does say something about this, but it would be easier
> to find out if it's described here.
>
> The first paragraph describing refspec format is moved down, so that
> the new first paragraph is about unspsecified refspec and the
> remaining paragraphs refspec format.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-push.txt | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 1398025..20bf2f5 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -33,13 +33,16 @@ OPTIONS[[OPTIONS]]
>  	of a remote (see the section <<REMOTES,REMOTES>> below).
>  
>  <refspec>...::
> -	The format of a <refspec> parameter is an optional plus
> -	`+`, followed by the source ref <src>, followed
> -	by a colon `:`, followed by the destination ref <dst>.
> -	It is used to specify with what <src> object the <dst> ref
> -	in the remote repository is to be updated.  If not specified,
> -	the behavior of the command is controlled by the `push.default`
> -	configuration variable.
> +	Specify how how refs are pushed. If not specified and
> +	configuration variable `remote.<remote>.push` is set, the
> +	variable is used as default refspec. If not specified and the
> +	variable is not set, the behavior of the command is controlled
> +	by the `push.default` configuration variable.
> ++
> +The format of a <refspec> parameter is an optional plus `+`, followed
> +by the source ref <src>, followed by a colon `:`, followed by the
> +destination ref <dst>.  It is used to specify with what <src> object
> +the <dst> ref in the remote repository is to be updated.

I agree that saying what it is, what it does or what it is for
upfront (i.e. "Specifies what are pushed") before how it is spelled
is an improvement.  I however think describing "If not specified"
here was a mistake, and you are making it worse by burying the
description of what happens when <refspec>... are missing in the
middle of the description for <refspec>...

I would rather see this done in the direction the attached "how
about doing it this way" patch illustrates.  The way how "where" and
"what" are determined when the command line does not specify is the
proparty of the entire command, not of an individual parameter.


 Documentation/git-push.txt | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 1398025..31dad19 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -23,6 +23,17 @@ You can make interesting things happen to a repository
 every time you push into it, by setting up 'hooks' there.  See
 documentation for linkgit:git-receive-pack[1].
 
+When the user does not specify where to push with
+<repository> argument, branch.*.remote configuration for the current
+branch is consulted to determine where to push.  If the
+configuration is missing, it defaults to 'origin'.
+
+When the user does not specify what to push with
+<refspec>... arguments or --all/--mirror/--tags options, the command finds
+the default <refspec> by consulting remote.*.push configuration, and
+if it is not found, honors push.default configuration to decide what
+to push (See gitlink:git-config[1] for the meaning of push.default).
+
 
 OPTIONS[[OPTIONS]]
 ------------------
@@ -33,13 +44,10 @@ OPTIONS[[OPTIONS]]
 	of a remote (see the section <<REMOTES,REMOTES>> below).
 
 <refspec>...::
+	Specify what destination ref to update with what source object.
 	The format of a <refspec> parameter is an optional plus
-	`+`, followed by the source ref <src>, followed
+	`+`, followed by the source object <src>, followed
 	by a colon `:`, followed by the destination ref <dst>.
-	It is used to specify with what <src> object the <dst> ref
-	in the remote repository is to be updated.  If not specified,
-	the behavior of the command is controlled by the `push.default`
-	configuration variable.
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
@@ -66,10 +74,7 @@ the remote repository.
 The special refspec `:` (or `+:` to allow non-fast-forward updates)
 directs Git to push "matching" branches: for every branch that exists on
 the local side, the remote side is updated if a branch of the same name
-already exists on the remote side.  This is the default operation mode
-if no explicit refspec is found (that is neither on the command line
-nor in any Push line of the corresponding remotes file---see below) and
-no `push.default` configuration variable is set.
+already exists on the remote side.
 
 --all::
 	Instead of naming each ref to push, specifies that all

^ permalink raw reply related

* Re: Merging submodules - best merge-base
From: Heiko Voigt @ 2013-03-06 18:12 UTC (permalink / raw)
  To: Daniel Bratell; +Cc: git, Jens Lehmann
In-Reply-To: <op.ws2l3rgnrbppqq@cicero.linkoping.osa>

On Mon, Feb 25, 2013 at 05:44:05PM +0100, Daniel Bratell wrote:
> I can phrase this in two ways and I'll start with the short way:
> 
> Why does a merge of a git submodule use as merge-base the commit that was  
> active in the merge-base of the parent repo, rather than the merge-base of  
> the two commits that are being merged?
> 
> The long question is:
> 
> A submodule change can be merged, but only if the merge is a  
> "fast-forward" which I think is a fair demand, but currently it checks if  
> it's a fast-forward from a commit that might not be very interesting  
> anymore.
> 
> If two branches A and B split at a point when they used submodule commit  
> S1 (based on S), and both then switched to S2 (also based on S) and B then  
> switched to S21, then it's today not possible to merge B into A, despite  
> S21 being a descendant of S2 and you get a conflict and this warning:
> 
> warning: Failed to merge submodule S (commits don't follow merge-base)
> 
> (attempt at ASCII gfx:
> 
> Submodule tree:
> 
> S ---- S1
>    \
>     \ - S2 -- S21
> 
> Main tree:
> 
> A' (uses S1) --- A (uses S2)
>    \
>     \ --- B' (uses S2) -- B (uses S21)
> 
> 
> I would like it to end up as:
> 
> A' (uses S1) --- A (uses S2) ------------ A+ (uses S21)
>    \                                     /
>     \ --- B' (uses S2) -- B (uses S21)- /
> 
> And that should be legal since S21 is a descendant of S2.

So to summarize what you are requesting: You want a submodule merge be
two way in the view of the superproject and calculate the merge base
in the submodule from the two commits that are going to be merged?

It currently sounds logical but I have to think about it further and
whether that might break other use cases.

Cheers Heiko

^ permalink raw reply

* Re: feature suggestion: optimize common parts for checkout --conflict=diff3
From: Antoine Pelisse @ 2013-03-06 18:27 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: git, kernel
In-Reply-To: <20130306150548.GC15375@pengutronix.de>

>         git checkout --conflict=diff3 file

That's somehow unrelated, but shouldn't we have a "conflict" option to
git-merge as we have for git-checkout ?

With something like this (pasted into gmail):

diff --git a/builtin/merge.c b/builtin/merge.c
index 7c8922c..edad742 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -65,6 +65,7 @@ static int abort_current_merge;
 static int show_progress = -1;
 static int default_to_upstream;
 static const char *sign_commit;
+static char *conflict_style;

 static struct strategy all_strategy[] = {
  { "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {
  { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
   N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
  OPT_BOOLEAN(0, "overwrite-ignore", &overwrite_ignore, N_("update
ignored files (default)")),
+ OPT_STRING(0, "conflict", &conflict_style, N_("style"), N_("conflict
style (merge or diff3)")),
  OPT_END()
 };

@@ -1102,6 +1104,9 @@ int cmd_merge(int argc, const char **argv, const
char *prefix)
  if (verbosity < 0 && show_progress == -1)
  show_progress = 0;

+ if (conflict_style)
+ git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
+
  if (abort_current_merge) {
  int nargc = 2;
  const char *nargv[] = {"reset", "--merge", NULL};

^ permalink raw reply related

* Re: [PATCH v2 1/4] wt-status: split wt_status_state parsing function out
From: Junio C Hamano @ 2013-03-06 18:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Matthieu Moy, Jonathan Niedier
In-Reply-To: <1362572482-20570-2-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  wt-status.c | 52 +++++++++++++++++++++++++++++++---------------------
>  wt-status.h |  5 +++--
>  2 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef405d0..183aafe 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -970,7 +970,7 @@ static void show_bisect_in_progress(struct wt_status *s,
>   * Extract branch information from rebase/bisect
>   */
>  static void read_and_strip_branch(struct strbuf *sb,
> -				  const char **branch,
> +				  char **branch,
>  				  const char *path)
>  {
>  	unsigned char sha1[20];
> @@ -994,52 +994,62 @@ static void read_and_strip_branch(struct strbuf *sb,
>  		strbuf_addstr(sb, abbrev);
>  		*branch = sb->buf;
>  	} else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
> -		;
> +		*branch = NULL;
>  	else			/* bisect */
>  		*branch = sb->buf;
> +	if (*branch)
> +		*branch = xstrdup(*branch);
>  }

The reason why the original print_state() kept two strbufs in it was
because its use of the return value (in *branch) from this function
was private and it did not want to have to strdup anything.

With this change, I suspect that it is much saner to make this
function *not* take any external strbuf as input, because you are
always returning a piece of memory that belongs to the caller, or a
NULL.

In other words, with the new world order, wouldn't a saner function
signature be:

	static const char *read_and_strip_branch(const char **path);

after this patch?

Also I notice that the error-return cases of this function may be
wrong even before your patch.  Shouldn't it be doing *branch = NULL
(and 'return NULL' after the suggested change)?

^ permalink raw reply

* What I want rebase to do
From: Dale R. Worley @ 2013-03-06 18:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: trast, git
In-Reply-To: <7vppzex2pq.fsf@alter.siamese.dyndns.org>

This is how I see what rebase should do:

The simple case for rebase starts from

               P---Q---R---S master
                    \
                     A---B---C topic

Then "git checkout topic ; git rebase master" will change it to

               P---Q---R---S master
                            \
                             A'--B'--C' topic

A' is created by a three-way merge that can be symbolized

        Q-->S
        |   |
        v   v
        A-->A'

That is, Q, applying the changes from Q to A and the changes from Q to
S, becomes A'.

After that

        A-->A'
        |   |
        v   v
        B-->B'
        |   |
        v   v
        C-->C'

A more complex case is when there is a merge from an external source

               P---Q---R---S master
                    \
                     A---M---C topic
                        /
                    ---X

We want to produce

               P---Q---R---S master
                            \
                             A'--M'--C' topic
                                /
                            ---X

So we have to merge

        Q-->S
        |   |
        v   v
        A-->A'
        |   |
        v   v
        M-->M'
        |   |
        v   v
        C-->C'

Any "evil" changes in M will be in the changes A->M (along with the
changes introduced from X), and so they will be reincorporated in
A'->M'.  M' lists A' and X as its parents.  (And not M!)

If there is an internal merge in the topic branch, things look like
this

               P---Q---R---S master
                    \
                     \   B
                      \ / \
                       A   M---D topic
                        \ /
                         C

and we want to produce this

               P---Q---R---S master
                            \
                             \    B'
                              \  /  \
                               A'    M'--D' topic
                                 \  /
                                  C'

Which can be done with these merges


        Q-->S
        |   |
        v   v
        A-->A'        A-->A'
        |   |         |   | 
        v   v         v   v 
        B-->B'        C-->C'

There are two choices for constructing M' (which ought to produce the
same results under ordinary circumstances)

        B-->B'        C-->C'
        |   |         |   | 
        v   v         v   v 
        M-->M'        M-->M'

and finally

        M-->M'
        |   |
        v   v
        D-->D'

Dale

^ permalink raw reply

* Re: [PATCH v2 3/4] status: show more info than "currently not on any branch"
From: Junio C Hamano @ 2013-03-06 19:16 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Matthieu Moy, Jonathan Niedier
In-Reply-To: <1362572482-20570-4-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> +static void wt_status_get_detached_from(struct wt_status_state *state)
> +{
> +	struct grab_1st_switch_cbdata cb;
> +	struct commit *commit;
> +	unsigned char sha1[20];
> +	char *ref = NULL;
> +
> +	strbuf_init(&cb.buf, 0);
> +	if (for_each_recent_reflog_ent("HEAD", grab_1st_switch, 4096, &cb))
> +		for_each_reflog_ent("HEAD", grab_1st_switch, &cb);
> +	if (!cb.buf.len)
> +		return;

Is this correct?  What if the recent entries (i.e. the tail 4k) of
the HEAD reflog did not have *any* checkout?  Your callback never
returns non-zero, so as long as the HEAD reflog is sufficiently
long, for_each_recent_reflog_ent() will return 0 to signal success,
and you do not dig deeper by retrying the full reflog for HEAD,
missing the checkout that exists before the final 4k, no?

It should be more like this, I would think:

	for_each_recent_reflog_ent();
        if (!found)
        	for_each_reflog_ent();
	if (!found)
        	return;

Using cb.buf.len as the "found" marker may be correct, but I found
it a bit subtle to my taste, without explanation.  Adding an
explicit bit to "struct grab_1st_switch_cbdata" would be cleaner and
more resistant to future changes, I think.

> +
> +	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
> +	    (commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
> +	    !hashcmp(cb.nsha1, commit->object.sha1)) {

That feels unnecessarily expensive.  Why not hashcmp before checking
the type of the object to reject the case where the ref moved since
the last checkout early?

For that matter, does this even need to check the type of the object
that currently sits at the ref?  Isn't it sufficient to reject this
case by seeing if sha1 is the same as cb.nsha1?

^ permalink raw reply

* Re: feature suggestion: optimize common parts for checkout --conflict=diff3
From: Antoine Pelisse @ 2013-03-06 19:26 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: git, kernel
In-Reply-To: <20130306150548.GC15375@pengutronix.de>

> however the difference isn't that easy to spot any more. I expected
>
>         diff --cc file
>         index a07e697,5080129..0000000
>         --- a/file
>         +++ b/file
>         @@@ -12,7 -12,7 +12,12 @@@
>           12
>           13
>           14
>         ++<<<<<<< ours
>          +15
>         ++||||||| base
>         ++=======
>         + 16
>         ++>>>>>>> theirs
>           17
>           18
>           19

This is not correct, it would mean that 12, 13, 14, 17, 18, 19 are in
base, while they are not.

Cheers,
Antoine

^ permalink raw reply

* Re: feature suggestion: optimize common parts for checkout --conflict=diff3
From: Jeff King @ 2013-03-06 20:03 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Uwe Kleine-König, git, kernel
In-Reply-To: <CALWbr2xDYuCN4nd-UNxkAY8-EguYjHBYgfu1fLtOGhYZyRQg_A@mail.gmail.com>

On Wed, Mar 06, 2013 at 08:26:57PM +0100, Antoine Pelisse wrote:

> > however the difference isn't that easy to spot any more. I expected
> >
> >         diff --cc file
> >         index a07e697,5080129..0000000
> >         --- a/file
> >         +++ b/file
> >         @@@ -12,7 -12,7 +12,12 @@@
> >           12
> >           13
> >           14
> >         ++<<<<<<< ours
> >          +15
> >         ++||||||| base
> >         ++=======
> >         + 16
> >         ++>>>>>>> theirs
> >           17
> >           18
> >           19
> 
> This is not correct, it would mean that 12, 13, 14, 17, 18, 19 are in
> base, while they are not.

Yeah, I agree it is a bit of a lie, as you are not seeing the full
picture of what was in the base. That is why we intentionally dial down
the conflict simplification level when using diff3. If you apply this
patch to git:

diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 9e13b25..f381e0c 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -420,15 +420,6 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
 	int style = xmp->style;
 	int favor = xmp->favor;
 
-	if (style == XDL_MERGE_DIFF3) {
-		/*
-		 * "diff3 -m" output does not make sense for anything
-		 * more aggressive than XDL_MERGE_EAGER.
-		 */
-		if (XDL_MERGE_EAGER < level)
-			level = XDL_MERGE_EAGER;
-	}
-
 	c = changes = NULL;
 
 	while (xscr1 && xscr2) {

then it will produce the output that Uwe expects. While it can be
misleading, I also think it can make some conflicts (like this one) much
easier to understand. I don't see any reason we can't have a "zealous
diff3" mode to let people view this output, as long as it is not the
default.

-Peff

^ permalink raw reply related

* [PATCH] xdiff: implement a zealous diff3
From: Uwe Kleine-König @ 2013-03-06 20:36 UTC (permalink / raw)
  To: git; +Cc: kernel, Antoine Pelisse, Jeff King
In-Reply-To: <20130306200347.GA20312@sigill.intra.peff.net>

"zdiff3" is identical to ordinary diff3, only it allows more aggressive
compaction than diff3. This way the displayed base isn't necessary
technically correct, but still this mode might help resolving merge
conflicts between two near identical additions.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this patch implements what I want. Thanks to Jeff for pointing me to the
right code to modify.

Best regards
Uwe

 builtin/merge-file.c                   | 2 ++
 contrib/completion/git-completion.bash | 2 +-
 xdiff-interface.c                      | 2 ++
 xdiff/xdiff.h                          | 1 +
 xdiff/xmerge.c                         | 8 +++++++-
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index c0570f2..4ef86aa 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -32,6 +32,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_BOOLEAN('p', "stdout", &to_stdout, N_("send results to standard output")),
 		OPT_SET_INT(0, "diff3", &xmp.style, N_("use a diff3 based merge"), XDL_MERGE_DIFF3),
+		OPT_SET_INT(0, "zdiff3", &xmp.style, N_("use a zealous diff3 based merge"),
+				XDL_MERGE_ZEALOUS_DIFF3),
 		OPT_SET_INT(0, "ours", &xmp.favor, N_("for conflicts, use our version"),
 			    XDL_MERGE_FAVOR_OURS),
 		OPT_SET_INT(0, "theirs", &xmp.favor, N_("for conflicts, use their version"),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b62bec0..a0d887e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1091,7 +1091,7 @@ _git_checkout ()
 
 	case "$cur" in
 	--conflict=*)
-		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
+		__gitcomp "diff3 merge zdiff3" "" "${cur##--conflict=}"
 		;;
 	--*)
 		__gitcomp "
diff --git a/xdiff-interface.c b/xdiff-interface.c
index ecfa05f..a911c25 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -308,6 +308,8 @@ int git_xmerge_config(const char *var, const char *value, void *cb)
 			die("'%s' is not a boolean", var);
 		if (!strcmp(value, "diff3"))
 			git_xmerge_style = XDL_MERGE_DIFF3;
+		else if (!strcmp(value, "zdiff3"))
+			git_xmerge_style = XDL_MERGE_ZEALOUS_DIFF3;
 		else if (!strcmp(value, "merge"))
 			git_xmerge_style = 0;
 		else
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 219a3bb..9730c63 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -64,6 +64,7 @@ extern "C" {
 
 /* merge output styles */
 #define XDL_MERGE_DIFF3 1
+#define XDL_MERGE_ZEALOUS_DIFF3 2
 
 typedef struct s_mmfile {
 	char *ptr;
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 9e13b25..4772707 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -177,7 +177,7 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 	size += xdl_recs_copy(xe1, m->i1, m->chg1, 1,
 			      dest ? dest + size : NULL);
 
-	if (style == XDL_MERGE_DIFF3) {
+	if (style == XDL_MERGE_DIFF3 || style == XDL_MERGE_ZEALOUS_DIFF3) {
 		/* Shared preimage */
 		if (!dest) {
 			size += marker_size + 1 + marker3_size;
@@ -420,6 +420,12 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
 	int style = xmp->style;
 	int favor = xmp->favor;
 
+	/*
+	 * This is the only change between XDL_MERGE_DIFF3 and
+	 * XDL_MERGE_ZEALOUS_DIFF3. "zdiff3" isn't 100% technically correct (as
+	 * the base might be considerably simplified), but still it might help
+	 * interpreting conflicts between two big and near identical additions.
+	 */
 	if (style == XDL_MERGE_DIFF3) {
 		/*
 		 * "diff3 -m" output does not make sense for anything
-- 
1.8.2.rc2

^ permalink raw reply related

* Re: feature suggestion: optimize common parts for checkout --conflict=diff3
From: Junio C Hamano @ 2013-03-06 20:40 UTC (permalink / raw)
  To: Jeff King; +Cc: Antoine Pelisse, Uwe Kleine-König, git, kernel
In-Reply-To: <20130306200347.GA20312@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> then it will produce the output that Uwe expects. While it can be
> misleading,...

Misleading is one thing but in this case isn't it outright wrong?

If you remove <<< ours ||| portion from the combined diff output,
I would expect that the hunk will apply to the base, but that is no
longer true, no?

^ permalink raw reply

* Re: [PATCH] xdiff: implement a zealous diff3
From: Jeff King @ 2013-03-06 20:46 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: git, kernel, Antoine Pelisse
In-Reply-To: <1362602202-29749-1-git-send-email-u.kleine-koenig@pengutronix.de>

On Wed, Mar 06, 2013 at 09:36:42PM +0100, Uwe Kleine-König wrote:

> "zdiff3" is identical to ordinary diff3, only it allows more aggressive
> compaction than diff3. This way the displayed base isn't necessary
> technically correct, but still this mode might help resolving merge
> conflicts between two near identical additions.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

I think the patch is correct, assuming this is the interface we want.

It would be more flexible instead to have:

  1. user can configure zealous-level of merge via command-line or
     config (they cannot control it at all right now)

  2. when diff3 is used and no level is explicitly given, do not go
     above EAGER

  3. otherwise, respect the level given by the user, even if it is
     ZEALOUS

But that would involve a lot of refactoring. I don't know if it is worth
the effort (the bonus is that people can then set the level
independently, but I do not know that anyone ever wants to do that).

>  builtin/merge-file.c                   | 2 ++
>  contrib/completion/git-completion.bash | 2 +-
>  xdiff-interface.c                      | 2 ++
>  xdiff/xdiff.h                          | 1 +
>  xdiff/xmerge.c                         | 8 +++++++-
>  5 files changed, 13 insertions(+), 2 deletions(-)

I think this would need documentation not only to let users know about
the feature, but also to warn them of the caveats.

-Peff

^ permalink raw reply

* Re: feature suggestion: optimize common parts for checkout --conflict=diff3
From: Jeff King @ 2013-03-06 20:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, Uwe Kleine-König, git, kernel
In-Reply-To: <7vvc94p8hb.fsf@alter.siamese.dyndns.org>

On Wed, Mar 06, 2013 at 12:40:48PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > then it will produce the output that Uwe expects. While it can be
> > misleading,...
> 
> Misleading is one thing but in this case isn't it outright wrong?
> 
> If you remove <<< ours ||| portion from the combined diff output,
> I would expect that the hunk will apply to the base, but that is no
> longer true, no?

It shifts the concept of what is the "base" and what is the "conflict".
In Uwe's example, no, it would not apply to the single-line file that is
the true 3-way base. But it would apply to the content that is outside
of the hunk marker; we have changed the concept of what is in the base
and what is in the conflict by shrinking the conflict to its smallest
size. The same is true of the conflict markers produced in the non-diff3
case. It is a property of XDL_MERGE_ZEALOUS, not of the conflict style.

If your argument is "diff3 means something different than regular
conflict markers; it should have the property of being
machine-convertible into a patch, but regular markers do not", I'm not
sure I agree. It may be used that way, but I think it is mostly used in
git to give the reader more context when making a resolution. And
anyway, I think the proposed change would not be to change diff3, but to
introduce a new diff3-like format that also shrinks the hunk size, so it
would not hurt existing users of diff3.

-Peff

^ 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