[PATCH 0/3] Make git more user-friendly during a merge conflict

STALE3736d

8 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/3] Make git more user-friendly during a merge conflict

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:19

2/3: I've added advice.mergeHints to silent the messages that suggests "git
merge--abort".

3/3: I've added a warning message when users used "git reset" during a merge.
This warning will be printed if the user is in the middle of a merge. In future
releases, we'll change this into an error to prevent work tree from becoming a
mess.

Andrew Wong (3):
  wt-status: Make status messages more consistent with others
  merge: Advise user to use "git merge --abort" to abort merges
  reset: Print a warning when user uses "git reset" during a merge

 Documentation/config.txt |  3 +++
 advice.c                 |  2 ++
 advice.h                 |  1 +
 builtin/merge.c          |  6 ++++++
 builtin/reset.c          | 21 +++++++++++++++++++++
 wt-status.c              | 23 +++++++++++++----------
 6 files changed, 46 insertions(+), 10 deletions(-)

-- 
1.9.0.174.g6f75b8f

[PATCH 1/3] wt-status: Make status messages more consistent with others

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:19

This is mainly changing messages that say:
    run "git foo --bar"
to
    use "git foo --bar" to baz

Although the commands and flags are usually self-explanatory, this is
more consistent with other status messages, and gives some sort of
explanation to the user.

Signed-off-by: Andrew Wong <redacted>
---
 wt-status.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index a452407..9f2358a 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -899,13 +899,13 @@ static void show_merge_in_progress(struct wt_status *s,
 		status_printf_ln(s, color, _("You have unmerged paths."));
 		if (s->hints)
 			status_printf_ln(s, color,
-				_("  (fix conflicts and run \"git commit\")"));
+				_("  (fix conflicts and use \"git commit\" to conclude the merge)"));
 	} else {
 		status_printf_ln(s, color,
 			_("All conflicts fixed but you are still merging."));
 		if (s->hints)
 			status_printf_ln(s, color,
-				_("  (use \"git commit\" to conclude merge)"));
+				_("  (use \"git commit\" to conclude the merge)"));
 	}
 	wt_status_print_trailer(s);
 }

@@ -922,7 +922,7 @@ static void show_am_in_progress(struct wt_status *s,
 	if (s->hints) {
 		if (!state->am_empty_patch)
 			status_printf_ln(s, color,
-				_("  (fix conflicts and then run \"git am --continue\")"));
+				_("  (fix conflicts and then use \"git am --continue\" to continue)"));
 		status_printf_ln(s, color,
 			_("  (use \"git am --skip\" to skip this patch)"));
 		status_printf_ln(s, color,

@@ -994,7 +994,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 					 _("You are currently rebasing."));
 		if (s->hints) {
 			status_printf_ln(s, color,
-				_("  (fix conflicts and then run \"git rebase --continue\")"));
+				_("  (fix conflicts and then use \"git rebase --continue\" to continue)"));
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --skip\" to skip this patch)"));
 			status_printf_ln(s, color,

@@ -1011,7 +1011,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 					 _("You are currently rebasing."));
 		if (s->hints)
 			status_printf_ln(s, color,
-				_("  (all conflicts fixed: run \"git rebase --continue\")"));
+				_("  (all conflicts fixed: use \"git rebase --continue\" to continue)"));
 	} else if (split_commit_in_progress(s)) {
 		if (state->branch)
 			status_printf_ln(s, color,

@@ -1023,7 +1023,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 					 _("You are currently splitting a commit during a rebase."));
 		if (s->hints)
 			status_printf_ln(s, color,
-				_("  (Once your working directory is clean, run \"git rebase --continue\")"));
+				_("  (Once your working directory is clean, use \"git rebase --continue\" to continue)"));
 	} else {
 		if (state->branch)
 			status_printf_ln(s, color,

@@ -1052,10 +1052,10 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 	if (s->hints) {
 		if (has_unmerged(s))
 			status_printf_ln(s, color,
-				_("  (fix conflicts and run \"git cherry-pick --continue\")"));
+				_("  (fix conflicts and use \"git cherry-pick --continue\" to continue)"));
 		else
 			status_printf_ln(s, color,
-				_("  (all conflicts fixed: run \"git cherry-pick --continue\")"));
+				_("  (all conflicts fixed: use \"git cherry-pick --continue\" to continue)"));
 		status_printf_ln(s, color,
 			_("  (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)"));
 	}

@@ -1071,10 +1071,10 @@ static void show_revert_in_progress(struct wt_status *s,
 	if (s->hints) {
 		if (has_unmerged(s))
 			status_printf_ln(s, color,
-				_("  (fix conflicts and run \"git revert --continue\")"));
+				_("  (fix conflicts and use \"git revert --continue\" to continue)"));
 		else
 			status_printf_ln(s, color,
-				_("  (all conflicts fixed: run \"git revert --continue\")"));
+				_("  (all conflicts fixed: use \"git revert --continue\" to continue)"));
 		status_printf_ln(s, color,
 			_("  (use \"git revert --abort\" to cancel the revert operation)"));
 	}

-- 
1.9.0.174.g6f75b8f

[PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:19

During a merge, "--mixed" is most likely not what the user wants. Using
"--mixed" during a merge would leave the merged changes and new files
mixed in with the local changes. The user would have to manually clean
up the work tree, which is non-trivial. In future releases, we want to
make "git reset" error out when used in the middle of a merge. For now,
we simply print out a warning to the user.

Signed-off-by: Andrew Wong <redacted>
---
 builtin/reset.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/builtin/reset.c b/builtin/reset.c
index 4fd1c6c..04e8103 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -331,8 +331,29 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 					_(reset_type_names[reset_type]));
 	}
 	if (reset_type == NONE)
+	{
 		reset_type = MIXED; /* by default */
 
+		/* During a merge, "--mixed" is most likely not what the user
+		 * wants. Using "--mixed" during a merge would leave the merged
+		 * changes and new files mixed in with the local changes. The
+		 * user would have to manually clean up the work tree, which is
+		 * non-trivial. In future releases, we want to make "git reset"
+		 * error out when used in the middle of a merge. For now, we
+		 * simply print out a warning to the user. */
+		if (is_merge())
+			warning(_("You have used 'git reset' in the middle of a merge. 'git reset' defaults to\n"
+				  "'git reset --mixed', which means git will not clean up any merged changes and\n"
+				  "new files that were created in the work tree. It also becomes impossible for\n"
+				  "git to automatically clean up the work tree later, so you would have to clean\n"
+				  "up the work tree manually. To avoid this next time, you may want to use 'git\n"
+				  "reset --merge', or equivalently 'git merge --abort'.\n"
+				  "\n"
+				  "In future releases, using 'git reset' in the middle of a merge will result in\n"
+				  "an error."
+				 ));
+	}
+
 	if (reset_type != SOFT && reset_type != MIXED)
 		setup_work_tree();

-- 
1.9.0.174.g6f75b8f

[PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:19

Print message during "git merge" and "git status".

Add a new "mergeHints" advice to silence these messages.

Signed-off-by: Andrew Wong <redacted>
---
 Documentation/config.txt | 3 +++
 advice.c                 | 2 ++
 advice.h                 | 1 +
 builtin/merge.c          | 6 ++++++
 wt-status.c              | 3 +++
 5 files changed, 15 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 73904bc..936a20b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -196,6 +196,9 @@ advice.*::
 	rmHints::
 		In case of failure in the output of linkgit:git-rm[1],
 		show directions on how to proceed from the current state.
+	mergeHints::
+                Show directions on how to proceed from the current state in the
+                output of linkgit:git-merge[1].
 --
 
 core.fileMode::

diff --git a/advice.c b/advice.c
index 486f823..e910734 100644
--- a/advice.c
+++ b/advice.c

@@ -15,6 +15,7 @@ int advice_detached_head = 1;
 int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
 int advice_rm_hints = 1;
+int advice_merge_hints = 1;
 
 static struct {
 	const char *name;

@@ -35,6 +36,7 @@ static struct {
 	{ "setupstreamfailure", &advice_set_upstream_failure },
 	{ "objectnamewarning", &advice_object_name_warning },
 	{ "rmhints", &advice_rm_hints },
+	{ "mergehints", &advice_merge_hints },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }

diff --git a/advice.h b/advice.h
index 5ecc6c1..d337f1c 100644
--- a/advice.h
+++ b/advice.h

@@ -18,6 +18,7 @@ extern int advice_detached_head;
 extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
 extern int advice_rm_hints;
+extern int advice_merge_hints;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))

diff --git a/builtin/merge.c b/builtin/merge.c
index f0cf120..c55ac03 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c

@@ -805,6 +805,8 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
 		error("%s", err_msg);
 	fprintf(stderr,
 		_("Not committing merge; use 'git commit' to complete the merge.\n"));
+	if (advice_merge_hints)
+		printf(_("  (use \"git merge --abort\" to abort the merge)\n"));
 	write_merge_state(remoteheads);
 	exit(1);
 }

@@ -913,6 +915,8 @@ static int suggest_conflicts(int renormalizing)
 	rerere(allow_rerere_auto);
 	printf(_("Automatic merge failed; "
 			"fix conflicts and then commit the result.\n"));
+	if (advice_merge_hints)
+		printf(_("  (use \"git merge --abort\" to abort the merge)\n"));
 	return 1;
 }

@@ -1559,6 +1563,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (merge_was_ok)
 		fprintf(stderr, _("Automatic merge went well; "
 			"stopped before committing as requested\n"));
+	if (advice_merge_hints)
+		printf(_("  (use \"git merge --abort\" to abort the merge)\n"));
 	else
 		ret = suggest_conflicts(option_renormalize);

diff --git a/wt-status.c b/wt-status.c
index 9f2358a..3b30bf9 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -907,6 +907,9 @@ static void show_merge_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git commit\" to conclude the merge)"));
 	}
+	if (s->hints)
+		status_printf_ln(s, color,
+			_("  (use \"git merge --abort\" to abort the merge)"));
 	wt_status_print_trailer(s);
 }

-- 
1.9.0.174.g6f75b8f

Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge

From: Marc Branchaud <hidden>
Date: 2016-06-15 23:00:19

On 14-03-14 12:37 AM, Andrew Wong wrote:

During a merge, "--mixed" is most likely not what the user wants. Using
"--mixed" during a merge would leave the merged changes and new files
mixed in with the local changes. The user would have to manually clean
up the work tree, which is non-trivial. In future releases, we want to
make "git reset" error out when used in the middle of a merge. For now,
we simply print out a warning to the user.

I know this approach was suggested earlier, but given these dangers it seems
silly to give this big warning on a plain "git reset" but still go ahead and
do the things the warning talks about.

Is there any issue with changing "git reset" to error-out now but letting
"git reset --mixed" proceed?  Something like (note the reworded warning message):

$ git reset
Cowardly refusing to implicitly run 'git reset --mixed' during a merge.
This would not clean up any merged changes and would not remove any new
files that were created in the work tree.  It would also make it impossible
for git to automatically clean up the work tree later, so you would have to
clean up the work tree manually.
You probably meant to run 'git merge --abort' instead.
$ git reset --mixed   # Stoopid git!  I know what I'm doing!
$

This would mean that the 10% of git users who like to do "git reset" in the
middle of a conflicted merge will have to teach their fingers some extra
motions.  But these users are all veterans, and they can more easily type in
8 extra characters (6 with completion) than new users can recover from
accidentally misusing git-reset's power.

		M.

Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:19

On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud [off-list ref] wrote:

I know this approach was suggested earlier, but given these dangers it seems
silly to give this big warning on a plain "git reset" but still go ahead and
do the things the warning talks about.

Is there any issue with changing "git reset" to error-out now but letting
"git reset --mixed" proceed?  Something like (note the reworded warning message):

Yeah, I would have preferred to have "git reset" error out right now,
because the messed up work tree can be quite a pain to clean up. The
main argument for issuing the warning is about maintaining
compatibility.

For the users that really did mean "--merge", the warning is silly.
It's basically saying "We know that you're about to mess up your work
tree, but we let you mess up anyway. Learn the correct way so that you
don't mess up next time".

It actually doesn't seem too bad if we did make "git reset" to error
out (during a merge) right away. By erroring out, the command won't
cause some irreversible damage, and users don't lose data. Yes, it
breaks compatibility, but perhaps not in a bad way?

I'm really fine with either. Junio?

Re: [PATCH 0/3] Make git more user-friendly during a merge conflict

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:21

Andrew Wong [off-list ref] writes:

2/3: I've added advice.mergeHints to silent the messages that suggests "git
merge--abort".

3/3: I've added a warning message when users used "git reset" during a merge.
This warning will be printed if the user is in the middle of a merge. In future
releases, we'll change this into an error to prevent work tree from becoming a
mess.

Andrew Wong (3):
  wt-status: Make status messages more consistent with others
  merge: Advise user to use "git merge --abort" to abort merges
  reset: Print a warning when user uses "git reset" during a merge

 Documentation/config.txt |  3 +++
 advice.c                 |  2 ++
 advice.h                 |  1 +
 builtin/merge.c          |  6 ++++++
 builtin/reset.c          | 21 +++++++++++++++++++++
 wt-status.c              | 23 +++++++++++++----------
 6 files changed, 46 insertions(+), 10 deletions(-)

Has this series been tested with existing test suite?  I tentatively
queued it to 'pu' but then had to revert because many tests started
failing, causing me to redo the today's integration cycle for 'pu'
once again.

Re: [PATCH 0/3] Make git more user-friendly during a merge conflict

From: Andrew Wong <hidden>
Date: 2016-06-15 23:00:21

On Mon, Mar 17, 2014 at 7:04 PM, Junio C Hamano [off-list ref] wrote:

Has this series been tested with existing test suite?  I tentatively
queued it to 'pu' but then had to revert because many tests started
failing, causing me to redo the today's integration cycle for 'pu'
once again.

I tested it during RFC, but missed it when I sent it as patch. The
problem is here:

@@ -1559,6 +1563,8 @@ int cmd_merge(int argc, const char **argv, const

char *prefix)
        if (merge_was_ok)
                fprintf(stderr, _("Automatic merge went well; "
                        "stopped before committing as requested\n"));
+       if (advice_merge_hints)
+               printf(_("  (use \"git merge --abort\" to abort the merge)\n"));
        else
                ret = suggest_conflicts(option_
renormalize);

I'll fix the problem. Sorry about that.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help