git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Make git more user-friendly during a merge conflict
@ 2014-03-14  4:37 Andrew Wong
  2014-03-14  4:37 ` [PATCH 1/3] wt-status: Make status messages more consistent with others Andrew Wong
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Andrew Wong @ 2014-03-14  4:37 UTC (permalink / raw)
  To: git; +Cc: Andrew Wong

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] wt-status: Make status messages more consistent with others
  2014-03-14  4:37 [PATCH 0/3] Make git more user-friendly during a merge conflict Andrew Wong
@ 2014-03-14  4:37 ` Andrew Wong
  2014-03-17 21:51   ` Junio C Hamano
  2014-03-14  4:37 ` [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges Andrew Wong
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Andrew Wong @ 2014-03-14  4:37 UTC (permalink / raw)
  To: git; +Cc: Andrew Wong

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 <andrew.kw.w@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges
  2014-03-14  4:37 [PATCH 0/3] Make git more user-friendly during a merge conflict Andrew Wong
  2014-03-14  4:37 ` [PATCH 1/3] wt-status: Make status messages more consistent with others Andrew Wong
@ 2014-03-14  4:37 ` Andrew Wong
  2014-03-17 21:58   ` Junio C Hamano
  2014-03-14  4:37 ` [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge Andrew Wong
  2014-03-17 23:04 ` [PATCH 0/3] Make git more user-friendly during a merge conflict Junio C Hamano
  3 siblings, 1 reply; 15+ messages in thread
From: Andrew Wong @ 2014-03-14  4:37 UTC (permalink / raw)
  To: git; +Cc: Andrew Wong

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

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

Signed-off-by: Andrew Wong <andrew.kw.w@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14  4:37 [PATCH 0/3] Make git more user-friendly during a merge conflict Andrew Wong
  2014-03-14  4:37 ` [PATCH 1/3] wt-status: Make status messages more consistent with others Andrew Wong
  2014-03-14  4:37 ` [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges Andrew Wong
@ 2014-03-14  4:37 ` Andrew Wong
  2014-03-14 14:33   ` Marc Branchaud
  2014-03-17 21:54   ` Junio C Hamano
  2014-03-17 23:04 ` [PATCH 0/3] Make git more user-friendly during a merge conflict Junio C Hamano
  3 siblings, 2 replies; 15+ messages in thread
From: Andrew Wong @ 2014-03-14  4:37 UTC (permalink / raw)
  To: git; +Cc: Andrew Wong

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 <andrew.kw.w@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14  4:37 ` [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge Andrew Wong
@ 2014-03-14 14:33   ` Marc Branchaud
  2014-03-14 17:04     ` Andrew Wong
  2014-03-17 21:54   ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Marc Branchaud @ 2014-03-14 14:33 UTC (permalink / raw)
  To: Andrew Wong, git

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14 14:33   ` Marc Branchaud
@ 2014-03-14 17:04     ` Andrew Wong
  2014-03-14 20:55       ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Wong @ 2014-03-14 17:04 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git@vger.kernel.org, Junio C Hamano

On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud <marcnarc@xiplink.com> 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?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14 17:04     ` Andrew Wong
@ 2014-03-14 20:55       ` Junio C Hamano
  2014-03-14 21:35         ` Andrew Wong
  2014-03-15 19:23         ` Marc Branchaud
  0 siblings, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-03-14 20:55 UTC (permalink / raw)
  To: Andrew Wong; +Cc: Marc Branchaud, git@vger.kernel.org

Andrew Wong <andrew.kw.w@gmail.com> writes:

> On Fri, Mar 14, 2014 at 10:33 AM, Marc Branchaud <marcnarc@xiplink.com> 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".

I suspect that you meant "--mixed" instead of "--merge" here.

I do not agree with the "We know that you are about to mess up"
above.  Whoever is issuing that warning message may not know better
than the user who is running "reset".  As you wrote "most likely not
what the user wants" in your proposed log message, the only thing we
know is that it _often_ is a newbie mistake.

I recently needed to manually cherry-pick only one half of a patch,
and the way I did so was

	git show $that_commit >P.diff
        git apply -3 P.diff
        ... conflicts are expected; that is why I used -3 in the
        ... first place
        git reset
        git diff HEAD
	edit
        ... edit away the other half I did not want to cherry-pick
        ... while fixing the conflicted parts that happened to be
        ... in the part I did want to cherry-pick

"git cherry-pick --no-commit $that_commit" could have been used as
a replacement for the first two steps but being able to run the
"reset" without erroring out was an essential part to make this
workflow usable.

So I am OK with "eventually error out by default", but not OK with
"we know better than the user and will not allow it at all".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14 20:55       ` Junio C Hamano
@ 2014-03-14 21:35         ` Andrew Wong
  2014-03-15 19:23         ` Marc Branchaud
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Wong @ 2014-03-14 21:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marc Branchaud, git@vger.kernel.org

On Fri, Mar 14, 2014 at 4:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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".
>
> I suspect that you meant "--mixed" instead of "--merge" here.

No, I did mean "--merge". It's silly for inexperienced users because
it's too late to use "--merge" by the time they realized they should
not have used the default. The work tree has already become a mess. So
they'd immediately think "if git was smart enough to warn me about the
mess, why not prevent me from getting into the mess in the first
place?"

For the experienced users, they would understand the warning, because
they would be aware of the index, and the effect that "--mixed" and
"--merge" have on it.

> So I am OK with "eventually error out by default", but not OK with
> "we know better than the user and will not allow it at all".

Again, I didn't mean "we know better than the user". However, from a
new user's perspective, they won't understand why "git reset" gives
the warning, but still "knowingly" messes up their work tree.

And "we don't know better than the user" is exactly why I think we
should "eventually error out" rather than automatically switching to
"--merge". As Matthieu was saying, automatically switching to
"--merge" could discard conflict resolutions, which would be
undesirable. So it's better for git to error out then having git
decides what the user (probably) wants.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14 20:55       ` Junio C Hamano
  2014-03-14 21:35         ` Andrew Wong
@ 2014-03-15 19:23         ` Marc Branchaud
  1 sibling, 0 replies; 15+ messages in thread
From: Marc Branchaud @ 2014-03-15 19:23 UTC (permalink / raw)
  To: Junio C Hamano, Andrew Wong; +Cc: git@vger.kernel.org

On 14-03-14 04:55 PM, Junio C Hamano wrote:
>
> So I am OK with "eventually error out by default", but not OK with
> "we know better than the user and will not allow it at all".

Can I interpret that as you being OK with my proposed "Cowardly 
refusing" approach?

		M.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] wt-status: Make status messages more consistent with others
  2014-03-14  4:37 ` [PATCH 1/3] wt-status: Make status messages more consistent with others Andrew Wong
@ 2014-03-17 21:51   ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-03-17 21:51 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git

Andrew Wong <andrew.kw.w@gmail.com> writes:

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

"git foo --bar" is fine, but "to baz" was hard to read without first
realizing that 'baz' stands for some/any verb.  I think rephrasing
it to

	use "git foo --bar" to do baz

would reduce confusion.

> 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);
>  }

The above hunk makes sense.

At first glance, I felt that none of the remainder made much sense.
My reaction was: "git foo --continue" to continue?  What else could
the --continue option even mean?

The real value I see in these conversions is by saying "use this to
continue" instead of an unconditional "run this", it implies "*IF*
you wanted to continue, you can do this", meaning that user also has
the option of *not* continuing.  But the proposed update falls short
of realizing the full potential, if that is the value we are trying
to add.  I'd say

	fix conflicts and then use "git am --continue" if you want
	to continue.

or an even more explicit

	fix conflicts and then use "git am --continue" if you want
	to continue; or you can "git am --abort" to discontinue.

would be an improvement, but

	fix conflicts and then use "git am --continue" to continue

is probably not quite.

> @@ -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,

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge
  2014-03-14  4:37 ` [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge Andrew Wong
  2014-03-14 14:33   ` Marc Branchaud
@ 2014-03-17 21:54   ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-03-17 21:54 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git

Andrew Wong <andrew.kw.w@gmail.com> writes:

> 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 <andrew.kw.w@gmail.com>
> ---
>  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

Two style niggles here.

> +		 * 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"

"we want"?  Has any of us decided on that?

> +		 * 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();

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges
  2014-03-14  4:37 ` [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges Andrew Wong
@ 2014-03-17 21:58   ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-03-17 21:58 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git

Andrew Wong <andrew.kw.w@gmail.com> writes:

> Print message during "git merge" and "git status".
>
> Add a new "mergeHints" advice to silence these messages.

This sounds sensible.  Don't we want to have this one take effect on
the places where advice.resolveConflict is used in git-pull?
I.e. something like:

	do_we_advise=no
	if advice.resolveConflict is not set:
		if advice.mergeHints is set to false:
                	do_we_advise=no
		else:
                	do_we_advise=yes
	else:
        	do_we_advise=yes

        if do_we_advise == 'yes':
        	give advice in die_conflict and die_merge

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] Make git more user-friendly during a merge conflict
  2014-03-14  4:37 [PATCH 0/3] Make git more user-friendly during a merge conflict Andrew Wong
                   ` (2 preceding siblings ...)
  2014-03-14  4:37 ` [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge Andrew Wong
@ 2014-03-17 23:04 ` Junio C Hamano
  2014-03-17 23:25   ` Andrew Wong
  3 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2014-03-17 23:04 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git

Andrew Wong <andrew.kw.w@gmail.com> 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] Make git more user-friendly during a merge conflict
  2014-03-17 23:04 ` [PATCH 0/3] Make git more user-friendly during a merge conflict Junio C Hamano
@ 2014-03-17 23:25   ` Andrew Wong
  2014-03-19 22:30     ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Wong @ 2014-03-17 23:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org

On Mon, Mar 17, 2014 at 7:04 PM, Junio C Hamano <gitster@pobox.com> 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] Make git more user-friendly during a merge conflict
  2014-03-17 23:25   ` Andrew Wong
@ 2014-03-19 22:30     ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2014-03-19 22:30 UTC (permalink / raw)
  To: Andrew Wong; +Cc: git@vger.kernel.org

Andrew Wong <andrew.kw.w@gmail.com> writes:

> On Mon, Mar 17, 2014 at 7:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Has this series been tested with existing test suite? ...
> I tested it during RFC, but missed it when I sent it as patch.
> ...
> I'll fix the problem. Sorry about that.

Thanks.  Will hold onto the topic branch lest I forget, but will
keep it out of 'pu' in the meantime.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-03-19 22:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-14  4:37 [PATCH 0/3] Make git more user-friendly during a merge conflict Andrew Wong
2014-03-14  4:37 ` [PATCH 1/3] wt-status: Make status messages more consistent with others Andrew Wong
2014-03-17 21:51   ` Junio C Hamano
2014-03-14  4:37 ` [PATCH 2/3] merge: Advise user to use "git merge --abort" to abort merges Andrew Wong
2014-03-17 21:58   ` Junio C Hamano
2014-03-14  4:37 ` [PATCH 3/3] reset: Print a warning when user uses "git reset" during a merge Andrew Wong
2014-03-14 14:33   ` Marc Branchaud
2014-03-14 17:04     ` Andrew Wong
2014-03-14 20:55       ` Junio C Hamano
2014-03-14 21:35         ` Andrew Wong
2014-03-15 19:23         ` Marc Branchaud
2014-03-17 21:54   ` Junio C Hamano
2014-03-17 23:04 ` [PATCH 0/3] Make git more user-friendly during a merge conflict Junio C Hamano
2014-03-17 23:25   ` Andrew Wong
2014-03-19 22:30     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).