Any interest in 'git merge --continue' as a command

Any interest in 'git merge --continue' as a command

[Chris Packham]

I hit this at $dayjob recently.

A developer had got themselves into a confused state when needing to
resolve a merge conflict.

They knew about git rebase --continue (and git am and git cherry-pick)
but they were unsure how to "continue" a merge (it didn't help that
the advice saying to use 'git commit' was scrolling off the top of the
terminal). I know that using 'git commit' has been the standard way to
complete a merge but given other commands have a --continue should
merge have it as well?

Re: Any interest in 'git merge --continue' as a command

[Jeff King]

On Fri, Dec 09, 2016 at 08:57:58PM +1300, Chris Packham wrote:

> I hit this at $dayjob recently.
> 
> A developer had got themselves into a confused state when needing to
> resolve a merge conflict.
> 
> They knew about git rebase --continue (and git am and git cherry-pick)
> but they were unsure how to "continue" a merge (it didn't help that
> the advice saying to use 'git commit' was scrolling off the top of the
> terminal). I know that using 'git commit' has been the standard way to
> complete a merge but given other commands have a --continue should
> merge have it as well?

It seems like that would be in line with 35d2fffdb (Provide 'git merge
--abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
goal was providing consistency with other multi-command operations.

I assume it would _just_ run a vanilla "git commit", and not try to do
any trickery with updating the index (which could be disastrous).

-Peff

Re: Any interest in 'git merge --continue' as a command

[Jacob Keller]

On December 9, 2016 1:11:27 AM PST, Jeff King <peff@peff.net> wrote:
>On Fri, Dec 09, 2016 at 08:57:58PM +1300, Chris Packham wrote:
>
>> I hit this at $dayjob recently.
>> 
>> A developer had got themselves into a confused state when needing to
>> resolve a merge conflict.
>> 
>> They knew about git rebase --continue (and git am and git
>cherry-pick)
>> but they were unsure how to "continue" a merge (it didn't help that
>> the advice saying to use 'git commit' was scrolling off the top of
>the
>> terminal). I know that using 'git commit' has been the standard way
>to
>> complete a merge but given other commands have a --continue should
>> merge have it as well?
>
>It seems like that would be in line with 35d2fffdb (Provide 'git merge
>--abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
>goal was providing consistency with other multi-command operations.
>
>I assume it would _just_ run a vanilla "git commit", and not try to do
>any trickery with updating the index (which could be disastrous).
>
>-Peff

This makes sense to me.

Thanks,
Jake

Re: Any interest in 'git merge --continue' as a command

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> They knew about git rebase --continue (and git am and git cherry-pick)
>> but they were unsure how to "continue" a merge (it didn't help that
>> the advice saying to use 'git commit' was scrolling off the top of the
>> terminal). I know that using 'git commit' has been the standard way to
>> complete a merge but given other commands have a --continue should
>> merge have it as well?
>
> It seems like that would be in line with 35d2fffdb (Provide 'git merge
> --abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
> goal was providing consistency with other multi-command operations.
>
> I assume it would _just_ run a vanilla "git commit", and not try to do
> any trickery with updating the index (which could be disastrous).

If we were to have "merge --continue", I agree that it would be the
logical implementation.

There is nothing to "continue" in a stopped merge where Git asked
for help from the user, and because of that, I view the final "git
commit" as "concluding the merge", not "continuing".  "continue"
makes quite a lot of sense with rebase and cherry-pick A..B that
stopped; it concludes the current step and let it continue to
process the remainder.  So from that point of view, it somewhat
feels strange to call it "merge --continue", but it probably is just
me.

Re: Any interest in 'git merge --continue' as a command

[Chris Packham]

On Sat, Dec 10, 2016 at 8:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>>> They knew about git rebase --continue (and git am and git cherry-pick)
>>> but they were unsure how to "continue" a merge (it didn't help that
>>> the advice saying to use 'git commit' was scrolling off the top of the
>>> terminal). I know that using 'git commit' has been the standard way to
>>> complete a merge but given other commands have a --continue should
>>> merge have it as well?
>>
>> It seems like that would be in line with 35d2fffdb (Provide 'git merge
>> --abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
>> goal was providing consistency with other multi-command operations.
>>
>> I assume it would _just_ run a vanilla "git commit", and not try to do
>> any trickery with updating the index (which could be disastrous).
>
> If we were to have "merge --continue", I agree that it would be the
> logical implementation.
>
> There is nothing to "continue" in a stopped merge where Git asked
> for help from the user, and because of that, I view the final "git
> commit" as "concluding the merge", not "continuing".  "continue"
> makes quite a lot of sense with rebase and cherry-pick A..B that
> stopped; it concludes the current step and let it continue to
> process the remainder.  So from that point of view, it somewhat
> feels strange to call it "merge --continue", but it probably is just
> me.
>

Yeah I did think that --continue wasn't quite the right word. git
merge --conclude would probably be the most accurate.

Re: Any interest in 'git merge --continue' as a command

[Jeff King]

On Fri, Dec 09, 2016 at 11:16:52AM -0800, Junio C Hamano wrote:

> > It seems like that would be in line with 35d2fffdb (Provide 'git merge
> > --abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
> > goal was providing consistency with other multi-command operations.
> >
> > I assume it would _just_ run a vanilla "git commit", and not try to do
> > any trickery with updating the index (which could be disastrous).
> 
> If we were to have "merge --continue", I agree that it would be the
> logical implementation.
> 
> There is nothing to "continue" in a stopped merge where Git asked
> for help from the user, and because of that, I view the final "git
> commit" as "concluding the merge", not "continuing".  "continue"
> makes quite a lot of sense with rebase and cherry-pick A..B that
> stopped; it concludes the current step and let it continue to
> process the remainder.  So from that point of view, it somewhat
> feels strange to call it "merge --continue", but it probably is just
> me.

No, I think your reasoning makes sense. But I also think we've already
choosen to have "--continue" mean "conclude the current, and continue if
there is anything left" in other contexts (e.g., a single-item
cherry-pick). It's more vague, but I think it keeps the user's mental
model simpler if we provide a standard set of options for multi-step
commands (e.g., always "--continue/--abort/--skip", though there are
some like merge that omit "--skip" if it does not make sense).

-Peff

Re: Any interest in 'git merge --continue' as a command

[Jeff King]

On Sat, Dec 10, 2016 at 09:49:13PM +1300, Chris Packham wrote:

> > There is nothing to "continue" in a stopped merge where Git asked
> > for help from the user, and because of that, I view the final "git
> > commit" as "concluding the merge", not "continuing".  "continue"
> > makes quite a lot of sense with rebase and cherry-pick A..B that
> > stopped; it concludes the current step and let it continue to
> > process the remainder.  So from that point of view, it somewhat
> > feels strange to call it "merge --continue", but it probably is just
> > me.
> 
> Yeah I did think that --continue wasn't quite the right word. git
> merge --conclude would probably be the most accurate.

I'd be against giving it a subtly-different name. It's just going to
frustrate people who cannot remember when to use "--conclude" and when
it is "--continue". The strength of the proposal, IMHO, is that it
abstracts the idea of "go on to the next thing or finish" across many
commands.

-Peff

Re: Any interest in 'git merge --continue' as a command

[Jacob Keller]

On Sat, Dec 10, 2016 at 1:00 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Dec 10, 2016 at 09:49:13PM +1300, Chris Packham wrote:
>
>> > There is nothing to "continue" in a stopped merge where Git asked
>> > for help from the user, and because of that, I view the final "git
>> > commit" as "concluding the merge", not "continuing".  "continue"
>> > makes quite a lot of sense with rebase and cherry-pick A..B that
>> > stopped; it concludes the current step and let it continue to
>> > process the remainder.  So from that point of view, it somewhat
>> > feels strange to call it "merge --continue", but it probably is just
>> > me.
>>
>> Yeah I did think that --continue wasn't quite the right word. git
>> merge --conclude would probably be the most accurate.
>
> I'd be against giving it a subtly-different name. It's just going to
> frustrate people who cannot remember when to use "--conclude" and when
> it is "--continue". The strength of the proposal, IMHO, is that it
> abstracts the idea of "go on to the next thing or finish" across many
> commands.
>
> -Peff

Agreed. I think "continue" makes sense as the command had to "stop"
the merge so you could give input, and then you tell git to "continue"
which also happens to mean "finish the merge" and yes it may not be
100% accurate, but the point of adding "git merge --continue" is that
it simplifies the mental model between rebase, cherry-pick, and merge,
all of which stop and ask the user to resolve a conflict before
"continue"ing and finalizing that resolution.

Thanks,
Jake

Re: Any interest in 'git merge --continue' as a command

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> No, I think your reasoning makes sense. But I also think we've already
> choosen to have "--continue" mean "conclude the current, and continue if
> there is anything left" in other contexts (e.g., a single-item
> cherry-pick). It's more vague, but I think it keeps the user's mental
> model simpler if we provide a standard set of options for multi-step
> commands (e.g., always "--continue/--abort/--skip", though there are
> some like merge that omit "--skip" if it does not make sense).

Yup.  I know you know me well enough to know that I didn't mean to
say "oh this one needs to be called differently" ;-)  I just felt
that "--continue" in that context did not sit well.

[RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit'

[Chris Packham]

Teach 'git merge' the --continue option which allows 'continuing' a
merge by completing it. The traditional way of completing a merge after
resolving conflicts is to use 'git commit'. Now with commands like 'git
rebase' and 'git cherry-pick' having a '--continue' option adding such
an option to 'git merge' presents a consistent UI.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
So here is a quick patch that adds the --continue option. I need to add
some tests (suggestions for where to start are welcome).

 Documentation/git-merge.txt | 13 ++++++++++++-
 builtin/merge.c             | 17 ++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d5556..765b0f26e 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
+'git merge' --continue
 
 DESCRIPTION
 -----------
@@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
 discouraged: while possible, it may leave you in a state that is hard to
 back out of in the case of a conflict.
 
+The fourth syntax ("`git merge --continue`") can only be run after the
+merge has resulted in conflicts. 'git merge --continue' will take the
+currently staged changes and complete the merge.
 
 OPTIONS
 -------
@@ -99,6 +103,12 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
+--continue::
+	Take the currently staged changes and complete the merge.
++
+'git merge --continue' is equivalent to 'git commit' when
+`MERGE_HEAD` is present.
+
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
@@ -277,7 +287,8 @@ After seeing a conflict, you can do two things:
 
  * Resolve the conflicts.  Git will mark the conflicts in
    the working tree.  Edit the files into shape and
-   'git add' them to the index.  Use 'git commit' to seal the deal.
+   'git add' them to the index.  Use 'git merge --continue' to seal the
+   deal.
 
 You can work through the conflict with a number of tools:
 
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87..1ce18cbbe 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -65,6 +65,7 @@ static int option_renormalize;
 static int verbosity;
 static int allow_rerere_auto;
 static int abort_current_merge;
+static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
@@ -223,6 +224,8 @@ static struct option builtin_merge_options[] = {
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
+	OPT_BOOL(0, "continue", &continue_current_merge,
+		N_("continue the current in-progress merge")),
 	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
 		 N_("allow merging unrelated histories")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
@@ -739,7 +742,7 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
 	if (err_msg)
 		error("%s", err_msg);
 	fprintf(stderr,
-		_("Not committing merge; use 'git commit' to complete the merge.\n"));
+		_("Not committing merge; use 'git merge --continue' to complete the merge.\n"));
 	write_merge_state(remoteheads);
 	exit(1);
 }
@@ -1166,6 +1169,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		goto done;
 	}
 
+	if (continue_current_merge) {
+		int nargc = 1;
+		const char *nargv[] = {"commit", NULL};
+
+		if (!file_exists(git_path_merge_head()))
+			die(_("There is no merge in progress (MERGE_HEAD missing)."));
+
+		/* Invoke 'git commit' */
+		ret = cmd_commit(nargc, nargv, prefix);
+		goto done;
+	}
+
 	if (read_cache_unmerged())
 		die_resolve_conflict("merge");
 
-- 
2.11.0

Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit'

[Markus Hitter]

Am 12.12.2016 um 09:34 schrieb Chris Packham:
> Teach 'git merge' the --continue option which allows 'continuing' a
> merge by completing it. The traditional way of completing a merge after
> resolving conflicts is to use 'git commit'. Now with commands like 'git
> rebase' and 'git cherry-pick' having a '--continue' option adding such
> an option to 'git merge' presents a consistent UI.

Like.

While Junio is entirely right that this is redundant, the inner workings of Git are just voodoo for a (guessed) 95% of users out there, so a consistent UI is important.

>  DESCRIPTION
>  -----------
> @@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
>  discouraged: while possible, it may leave you in a state that is hard to
>  back out of in the case of a conflict.
>  
> +The fourth syntax ("`git merge --continue`") can only be run after the
> +merge has resulted in conflicts. 'git merge --continue' will take the
> +currently staged changes and complete the merge.

I think this should mention the equivalence to 'git commit'.


Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit'

[Jeff King]

On Mon, Dec 12, 2016 at 09:34:13PM +1300, Chris Packham wrote:

> Teach 'git merge' the --continue option which allows 'continuing' a
> merge by completing it. The traditional way of completing a merge after
> resolving conflicts is to use 'git commit'. Now with commands like 'git
> rebase' and 'git cherry-pick' having a '--continue' option adding such
> an option to 'git merge' presents a consistent UI.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> So here is a quick patch that adds the --continue option. I need to add
> some tests (suggestions for where to start are welcome).

I'm not sure if there's much to test besides concluding a successful
merge, and possibly some error cases where --continue should complain.
Probably that could go at the end of t7600.

> @@ -1166,6 +1169,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		goto done;
>  	}
>  
> +	if (continue_current_merge) {
> +		int nargc = 1;
> +		const char *nargv[] = {"commit", NULL};
> +
> +		if (!file_exists(git_path_merge_head()))
> +			die(_("There is no merge in progress (MERGE_HEAD missing)."));
> +
> +		/* Invoke 'git commit' */
> +		ret = cmd_commit(nargc, nargv, prefix);
> +		goto done;
> +	}
> +

I know this block is just adapted from the "--abort" one above, but
should both of these complain when other arguments are given? I can't
imagine what the user might mean with "git merge --no-commit
--continue", but probably it should be an error.  :)

-Peff

Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit'

[Chris Packham]

On Mon, Dec 12, 2016 at 10:02 PM, Markus Hitter <mah@jump-ing.de> wrote:
> Am 12.12.2016 um 09:34 schrieb Chris Packham:
>> Teach 'git merge' the --continue option which allows 'continuing' a
>> merge by completing it. The traditional way of completing a merge after
>> resolving conflicts is to use 'git commit'. Now with commands like 'git
>> rebase' and 'git cherry-pick' having a '--continue' option adding such
>> an option to 'git merge' presents a consistent UI.
>
> Like.
>
> While Junio is entirely right that this is redundant, the inner workings of Git are just voodoo for a (guessed) 95% of users out there, so a consistent UI is important.
>
>>  DESCRIPTION
>>  -----------
>> @@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
>>  discouraged: while possible, it may leave you in a state that is hard to
>>  back out of in the case of a conflict.
>>
>> +The fourth syntax ("`git merge --continue`") can only be run after the
>> +merge has resulted in conflicts. 'git merge --continue' will take the
>> +currently staged changes and complete the merge.
>
> I think this should mention the equivalence to 'git commit'.
>

It is mentioned in the OPTIONS section where the --continue option is
documented. I could move it here but the OPTIONS section is where the
--abort synonym also has a reference to git reset --merge.

>
> Markus
>
> --
> - - - - - - - - - - - - - - - - - - -
> Dipl. Ing. (FH) Markus Hitter
> http://www.jump-ing.de/

[PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'

[Chris Packham]

Teach 'git merge' the --continue option which allows 'continuing' a
merge by completing it. The traditional way of completing a merge after
resolving conflicts is to use 'git commit'. Now with commands like 'git
rebase' and 'git cherry-pick' having a '--continue' option adding such
an option to 'git merge' presents a consistent UI.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

Notes:
    Changes in v2:
    - add --continue to builtin_merge_usage
    - verify that no other arguments are present when --continue is used.
    - add basic test

 Documentation/git-merge.txt | 13 ++++++++++++-
 builtin/merge.c             | 22 +++++++++++++++++++++-
 t/t7600-merge.sh            |  8 ++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d5556..765b0f26e 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
+'git merge' --continue
 
 DESCRIPTION
 -----------
@@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
 discouraged: while possible, it may leave you in a state that is hard to
 back out of in the case of a conflict.
 
+The fourth syntax ("`git merge --continue`") can only be run after the
+merge has resulted in conflicts. 'git merge --continue' will take the
+currently staged changes and complete the merge.
 
 OPTIONS
 -------
@@ -99,6 +103,12 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
+--continue::
+	Take the currently staged changes and complete the merge.
++
+'git merge --continue' is equivalent to 'git commit' when
+`MERGE_HEAD` is present.
+
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
@@ -277,7 +287,8 @@ After seeing a conflict, you can do two things:
 
  * Resolve the conflicts.  Git will mark the conflicts in
    the working tree.  Edit the files into shape and
-   'git add' them to the index.  Use 'git commit' to seal the deal.
+   'git add' them to the index.  Use 'git merge --continue' to seal the
+   deal.
 
 You can work through the conflict with a number of tools:
 
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87..379685223 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
 	N_("git merge [<options>] [<commit>...]"),
 	N_("git merge [<options>] <msg> HEAD <commit>"),
 	N_("git merge --abort"),
+	N_("git merge --continue"),
 	NULL
 };
 
@@ -65,6 +66,7 @@ static int option_renormalize;
 static int verbosity;
 static int allow_rerere_auto;
 static int abort_current_merge;
+static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
@@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
+	OPT_BOOL(0, "continue", &continue_current_merge,
+		N_("continue the current in-progress merge")),
 	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
 		 N_("allow merging unrelated histories")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
@@ -739,7 +743,7 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
 	if (err_msg)
 		error("%s", err_msg);
 	fprintf(stderr,
-		_("Not committing merge; use 'git commit' to complete the merge.\n"));
+		_("Not committing merge; use 'git merge --continue' to complete the merge.\n"));
 	write_merge_state(remoteheads);
 	exit(1);
 }
@@ -1166,6 +1170,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		goto done;
 	}
 
+	if (continue_current_merge) {
+		int nargc = 1;
+		const char *nargv[] = {"commit", NULL};
+
+		if (argc)
+			usage_msg_opt("--continue expects no arguments",
+			      builtin_merge_usage, builtin_merge_options);
+
+		if (!file_exists(git_path_merge_head()))
+			die(_("There is no merge in progress (MERGE_HEAD missing)."));
+
+		/* Invoke 'git commit' */
+		ret = cmd_commit(nargc, nargv, prefix);
+		goto done;
+	}
+
 	if (read_cache_unmerged())
 		die_resolve_conflict("merge");
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 85248a14b..44b34ef3a 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,7 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge -s foobar c1 &&
 	test_must_fail git merge -s=foobar c1 &&
 	test_must_fail git merge -m &&
+	test_must_fail git merge --continue foobar &&
 	test_must_fail git merge
 '
 
@@ -763,4 +764,11 @@ test_expect_success 'merge nothing into void' '
 	)
 '
 
+test_expect_success 'merge can be completed with --continue' '
+	git reset --hard c0 &&
+	git merge --no-ff --no-commit c1 &&
+	git merge --continue &&
+	verify_parents $c0 $c1
+'
+
 test_done
-- 
2.11.0.24.ge6920cf

[PATCHv2 2/2] completion: add --continue option for merge

[Chris Packham]

Add 'git merge --continue' option when completing.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

Notes:
    Changes in v2:
    - new.

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..1f97ffae1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1552,7 +1552,7 @@ _git_merge ()
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_merge_options
-			--rerere-autoupdate --no-rerere-autoupdate --abort"
+			--rerere-autoupdate --no-rerere-autoupdate --abort --continue"
 		return
 	esac
 	__gitcomp_nl "$(__git_refs)"
-- 
2.11.0.24.ge6920cf

Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'

[Jeff King]

On Tue, Dec 13, 2016 at 09:48:58PM +1300, Chris Packham wrote:

> +	if (continue_current_merge) {
> +		int nargc = 1;
> +		const char *nargv[] = {"commit", NULL};
> +
> +		if (argc)
> +			usage_msg_opt("--continue expects no arguments",
> +			      builtin_merge_usage, builtin_merge_options);

This checks that we don't have:

  git merge --continue foobar

but still allows:

  git merge --continue --some-option

because parse_options() decrements argc.

It would be insane to check individually which options might have been
set. But I wonder if we could do something like:

  int orig_argc = argc;
  ...
  argc = parse_options(argc, argv, ...);

  if (continue_current_merge) {
	if (orig_argc != 1) /* maybe 2, to account for argv[0] ? */
		usage_msg_opt("--continue expects no arguments", ...);
  }

That gets trickier if there ever is an option that's OK to use with
--continue. We might want to forward along "--quiet", for example. On
the other hand, we silently ignore it now, so maybe it is better to
complain and then let --quiet get added later if somebody cares.

Whatever we do here, I think "--abort" should get the same treatment
(probably as a separate patch).

> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 85248a14b..44b34ef3a 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -154,6 +154,7 @@ test_expect_success 'test option parsing' '
>  	test_must_fail git merge -s foobar c1 &&
>  	test_must_fail git merge -s=foobar c1 &&
>  	test_must_fail git merge -m &&
> +	test_must_fail git merge --continue foobar &&
>  	test_must_fail git merge
>  '

Your tests look good, though obviously if you check for options above,
that should be covered in this test.

-Peff

Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit'

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> +'git merge' --continue
>  
>  DESCRIPTION
>  -----------
> @@ -61,6 +62,9 @@ reconstruct the original (pre-merge) changes. Therefore:
>  discouraged: while possible, it may leave you in a state that is hard to
>  back out of in the case of a conflict.
>  
> +The fourth syntax ("`git merge --continue`") can only be run after the
> +merge has resulted in conflicts.

OK.  I can see that the code refuses if there is no MERGE_HEAD, so
"can only be run" is ensured correctly.

> 'git merge --continue' will take the
> +currently staged changes and complete the merge.

For Git-savvy folks, this may be sufficient to tell that they are
expected to resolve conflicts in the working tree and register the
resolusion by doing "git add" before running "git merge --continue",
but I wonder if that is clear enough for new readers.

The same comment applies to the option description below.  I suspect
that it is better to remove the last sentence above, leaving "4th
one can be run only with MERGE_HEAD" here, and enhance the
explanation in the option description (see below).

>  OPTIONS
>  -------
> @@ -99,6 +103,12 @@ commit or stash your changes before running 'git merge'.
>  'git merge --abort' is equivalent to 'git reset --merge' when
>  `MERGE_HEAD` is present.
>  
> +--continue::
> +	Take the currently staged changes and complete the merge.
> ++
> +'git merge --continue' is equivalent to 'git commit' when
> +`MERGE_HEAD` is present.
> +

These two sentences are even more technical and unfriendly to new
readers, I am afraid.  How about giving a hint by referring to an
existing section, like this?

    --continue::
        After a "git merge" stops due to conflicts you can conclude
        the merge by running "git merge --continue" (see "How to
        resolve conflicts" section below).

> @@ -277,7 +287,8 @@ After seeing a conflict, you can do two things:
>  
>   * Resolve the conflicts.  Git will mark the conflicts in
>     the working tree.  Edit the files into shape and
> -   'git add' them to the index.  Use 'git commit' to seal the deal.
> +   'git add' them to the index.  Use 'git merge --continue' to seal the
> +   deal.

Why do we want to make it harder to discover "git commit" here?
I would understand:

	... Use 'git commit' to conclude (you can also say 'git
	merge --continue').

though.  After all, we are merely introducing a synonym for those
who want to type more.  There is no plan to deprecate the use of
'git commit', which is a perfectly reasonable way to conclude an
interrupted merge, that has worked well for us for the past 10 years
and still works.

> @@ -65,6 +66,7 @@ static int option_renormalize;
>  static int verbosity;
>  static int allow_rerere_auto;
>  static int abort_current_merge;
> +static int continue_current_merge;
>  static int allow_unrelated_histories;
>  static int show_progress = -1;
>  static int default_to_upstream = 1;
> @@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
>  	OPT__VERBOSITY(&verbosity),
>  	OPT_BOOL(0, "abort", &abort_current_merge,
>  		N_("abort the current in-progress merge")),
> +	OPT_BOOL(0, "continue", &continue_current_merge,
> +		N_("continue the current in-progress merge")),
>  	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
>  		 N_("allow merging unrelated histories")),
>  	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
> @@ -739,7 +743,7 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
>  	if (err_msg)
>  		error("%s", err_msg);
>  	fprintf(stderr,
> -		_("Not committing merge; use 'git commit' to complete the merge.\n"));
> +		_("Not committing merge; use 'git merge --continue' to complete the merge.\n"));

Likewise.  I do not see a need to change this one at all.

> @@ -1166,6 +1170,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		goto done;
>  	}
>  
> +	if (continue_current_merge) {
> +		int nargc = 1;
> +		const char *nargv[] = {"commit", NULL};
> +
> +		if (argc)
> +			usage_msg_opt("--continue expects no arguments",
> +			      builtin_merge_usage, builtin_merge_options);

Peff already commented on "what about other options?", and I think
his "check the number of args before parse-options ran to ensure
that the '--abort' or '--continue' was the only thing" is probably
a workable hack.

The "right" way to fix it would be way too involved to be worth for
just this single change (and also fixing "--abort").  Just thinking
aloud:

 * Update parse-options API to:

   - extend "struct option" with one field that holds what "command
     modes" the option the "struct option" describes is incompatible
     with.

   - make parse_options() to keep track of the set of command modes
     that are compatible with the options seen so far, and complain
     if an option that is not compatible with the command mode is
     given.

 * Use the above facility to update "git merge" so that --abort and
   --continue becomes OPT_CMDMODE.

Then, the updated parse_options() would:

 - start by making the "incompatible command modes" an empty set.

 - while it processes each option on the command line:

   - if it is not an OPTION_CMDMODE, add the set of command modes
     that are incompatible with the option to the "incompatible
     command modes".

   - if it is an OPTION_CMDMODE and we already saw another
     OPTION_CMDMODE, error out (we already do this).

 - after all options are read, check the final command mode and see
   if that is in "incompatible command modes".

You can mark almost all options "git merge" takes except a selected
few like "--quiet" as incompatible with "--abort" and "--continue"
and let parse_options() catch incompatible options.  Of course you
still need to check argc for non-option here.

[PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Chris Packham]

Teach 'git merge' the --continue option which allows 'continuing' a
merge by completing it. The traditional way of completing a merge after
resolving conflicts is to use 'git commit'. Now with commands like 'git
rebase' and 'git cherry-pick' having a '--continue' option adding such
an option to 'git merge' presents a consistent UI.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v2:
- add --continue to builtin_merge_usage
- verify that no other arguments are present when --continue is used.
- add basic test
Changes in v3:
- check for other options in addtion to arguments, add test for this case
- re-instate references to 'git commit' that were removed in v2
- re-work documentation

 Documentation/git-merge.txt |  8 ++++++++
 builtin/merge.c             | 21 +++++++++++++++++++++
 t/t7600-merge.sh            |  9 +++++++++
 3 files changed, 38 insertions(+)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d5556..ca3c27b88 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
+'git merge' --continue
 
 DESCRIPTION
 -----------
@@ -61,6 +62,8 @@ reconstruct the original (pre-merge) changes. Therefore:
 discouraged: while possible, it may leave you in a state that is hard to
 back out of in the case of a conflict.
 
+The fourth syntax ("`git merge --continue`") can only be run after the
+merge has resulted in conflicts.
 
 OPTIONS
 -------
@@ -99,6 +102,11 @@ commit or stash your changes before running 'git merge'.
 'git merge --abort' is equivalent to 'git reset --merge' when
 `MERGE_HEAD` is present.
 
+--continue::
+	After a 'git merge' stops due to conflicts you can conclude the
+	merge by running 'git merge --continue' (see "HOW TO RESOLVE
+	CONFLICTS" section below).
+
 <commit>...::
 	Commits, usually other branch heads, to merge into our branch.
 	Specifying more than one commit will create a merge with
diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87..836ec281b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,6 +46,7 @@ static const char * const builtin_merge_usage[] = {
 	N_("git merge [<options>] [<commit>...]"),
 	N_("git merge [<options>] <msg> HEAD <commit>"),
 	N_("git merge --abort"),
+	N_("git merge --continue"),
 	NULL
 };
 
@@ -65,6 +66,7 @@ static int option_renormalize;
 static int verbosity;
 static int allow_rerere_auto;
 static int abort_current_merge;
+static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
@@ -223,6 +225,8 @@ static struct option builtin_merge_options[] = {
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
+	OPT_BOOL(0, "continue", &continue_current_merge,
+		N_("continue the current in-progress merge")),
 	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
 		 N_("allow merging unrelated histories")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
@@ -1125,6 +1129,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list *remoteheads, *p;
 	void *branch_to_free;
+	int orig_argc = argc;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_merge_usage, builtin_merge_options);
@@ -1166,6 +1171,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		goto done;
 	}
 
+	if (continue_current_merge) {
+		int nargc = 1;
+		const char *nargv[] = {"commit", NULL};
+
+		if (orig_argc != 2)
+			usage_msg_opt("--continue expects no arguments",
+			      builtin_merge_usage, builtin_merge_options);
+
+		if (!file_exists(git_path_merge_head()))
+			die(_("There is no merge in progress (MERGE_HEAD missing)."));
+
+		/* Invoke 'git commit' */
+		ret = cmd_commit(nargc, nargv, prefix);
+		goto done;
+	}
+
 	if (read_cache_unmerged())
 		die_resolve_conflict("merge");
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 85248a14b..682139c4e 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,8 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge -s foobar c1 &&
 	test_must_fail git merge -s=foobar c1 &&
 	test_must_fail git merge -m &&
+	test_must_fail git merge --continue foobar &&
+	test_must_fail git merge --continue --quiet &&
 	test_must_fail git merge
 '
 
@@ -763,4 +765,11 @@ test_expect_success 'merge nothing into void' '
 	)
 '
 
+test_expect_success 'merge can be completed with --continue' '
+	git reset --hard c0 &&
+	git merge --no-ff --no-commit c1 &&
+	git merge --continue &&
+	verify_parents $c0 $c1
+'
+
 test_done
-- 
2.11.0.24.ge6920cf

[PATCH 2/3] completion: add --continue option for merge

[Chris Packham]

Add 'git merge --continue' option when completing.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v2:
- new
Changes in v3:
- none

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..1f97ffae1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1552,7 +1552,7 @@ _git_merge ()
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_merge_options
-			--rerere-autoupdate --no-rerere-autoupdate --abort"
+			--rerere-autoupdate --no-rerere-autoupdate --abort --continue"
 		return
 	esac
 	__gitcomp_nl "$(__git_refs)"
-- 
2.11.0.24.ge6920cf

[PATCH 3/3] merge: Ensure '--abort' option takes no arguments

[Chris Packham]

Like '--continue', the '--abort' option doesn't make any sense with
other options or arguments to 'git merge' so ensure that none are
present.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
Changes in v3:
- new

 builtin/merge.c  | 4 ++++
 t/t7600-merge.sh | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/builtin/merge.c b/builtin/merge.c
index 836ec281b..668aaffb8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1163,6 +1163,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		int nargc = 2;
 		const char *nargv[] = {"reset", "--merge", NULL};
 
+		if (orig_argc != 2)
+			usage_msg_opt("--abort expects no arguments",
+			      builtin_merge_usage, builtin_merge_options);
+
 		if (!file_exists(git_path_merge_head()))
 			die(_("There is no merge to abort (MERGE_HEAD missing)."));
 
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 682139c4e..2ebda509a 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -154,6 +154,8 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge -s foobar c1 &&
 	test_must_fail git merge -s=foobar c1 &&
 	test_must_fail git merge -m &&
+	test_must_fail git merge --abort foobar &&
+	test_must_fail git merge --abort --quiet &&
 	test_must_fail git merge --continue foobar &&
 	test_must_fail git merge --continue --quiet &&
 	test_must_fail git merge
-- 
2.11.0.24.ge6920cf

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Jeff King]

On Wed, Dec 14, 2016 at 09:37:55PM +1300, Chris Packham wrote:

> +	if (continue_current_merge) {
> +		int nargc = 1;
> +		const char *nargv[] = {"commit", NULL};
> +
> +		if (orig_argc != 2)
> +			usage_msg_opt("--continue expects no arguments",
> +			      builtin_merge_usage, builtin_merge_options);

This message should probably be inside a _() for translation.

I noticed when running it that the output looks funny:

  $ git merge --continue foo
  --continue expects no arguments

  usage: [...]

I was going to suggest adding something like "fatal:" here, but I
actually think it should be the responsibility of usage_msg_opt().
Looking at its other callers, they would all benefit. I posted a
patch:

  http://public-inbox.org/git/20161214151009.4wdzjb44f6aki5ug@sigill.intra.peff.net/

I also wondered what it would look like to support "--quiet" on top of
this.  I don't care that much about it in particular, but I just want to
make sure we're not painting ourselves into a corner.

Here's what I came up with;

diff --git a/builtin/merge.c b/builtin/merge.c
index 668aaffb8..b13523ce9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1160,10 +1160,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		show_progress = 0;
 
 	if (abort_current_merge) {
-		int nargc = 2;
-		const char *nargv[] = {"reset", "--merge", NULL};
+		int acceptable_arguments = 2; /* argv[0] plus --abort */
+		struct argv_array nargv = ARGV_ARRAY_INIT;
 
-		if (orig_argc != 2)
+		argv_array_pushl(&nargv, "reset", "--merge", NULL);
+		if (verbosity < 0) {
+			acceptable_arguments++;
+			argv_array_push(&nargv, "--quiet");
+		}
+
+		if (orig_argc != acceptable_arguments)
 			usage_msg_opt("--abort expects no arguments",
 			      builtin_merge_usage, builtin_merge_options);
 
@@ -1171,15 +1177,22 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die(_("There is no merge to abort (MERGE_HEAD missing)."));
 
 		/* Invoke 'git reset --merge' */
-		ret = cmd_reset(nargc, nargv, prefix);
+		ret = cmd_reset(nargv.argc, nargv.argv, prefix);
+		argv_array_clear(&nargv);
 		goto done;
 	}
 
 	if (continue_current_merge) {
-		int nargc = 1;
-		const char *nargv[] = {"commit", NULL};
+		int acceptable_arguments = 2; /* argv[0] plus --abort */
+		struct argv_array nargv = ARGV_ARRAY_INIT;
+
+		argv_array_push(&nargv, "commit");
+		if (verbosity < 0) {
+			acceptable_arguments++;
+			argv_array_push(&nargv, "--quiet");
+		}
 
-		if (orig_argc != 2)
+		if (orig_argc != acceptable_arguments)
 			usage_msg_opt("--continue expects no arguments",
 			      builtin_merge_usage, builtin_merge_options);
 
@@ -1187,7 +1200,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die(_("There is no merge in progress (MERGE_HEAD missing)."));
 
 		/* Invoke 'git commit' */
-		ret = cmd_commit(nargc, nargv, prefix);
+		ret = cmd_commit(nargv.argc, nargv.argv, prefix);
+		argv_array_clear(&nargv);
 		goto done;
 	}
 

So not too bad (and you could probably refactor it to avoid some of the
duplication). Though it does get some obscure cases wrong, like:

  git merge --continue --verbose --quiet

I dunno. Maybe I am leading you down a rabbit hole, and we should just
live with silently ignoring useless options. I looked at what
cherry-pick does for this case, and its verify_opt_compatible is
somewhat scary from a maintenance standpoint. It's a whitelist, not a
blacklist, so it's easy to forget options (and it looks like "git
cherry-pick --abort -Sfoo" is missed, for example).

-Peff

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> So not too bad (and you could probably refactor it to avoid some of the
> duplication). Though it does get some obscure cases wrong, like:
>
>   git merge --continue --verbose --quiet
>
> I dunno. Maybe I am leading you down a rabbit hole, and we should just
> live with silently ignoring useless options.

I think you need to handle this in parse-options API if you really
wanted to do this correctly.  

<xmqq60mn671x.fsf@gitster.mtv.corp.google.com> may serve as a
reasonable outline for building one.

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Junio C Hamano]

The last one 3/3 is a nice touch that makes sure that we do not
forget what we discovered during the discussion.  Very much
appreciated.

Will queue.  Thanks.

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Chris Packham]

On Thu, Dec 15, 2016 at 7:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> The last one 3/3 is a nice touch that makes sure that we do not
> forget what we discovered during the discussion.  Very much
> appreciated.
>
> Will queue.  Thanks.

Did you want me to send a v4 to mark the strings for translation or
will you apply a fixup your end?

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> On Thu, Dec 15, 2016 at 7:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> The last one 3/3 is a nice touch that makes sure that we do not
>> forget what we discovered during the discussion.  Very much
>> appreciated.
>>
>> Will queue.  Thanks.
>
> Did you want me to send a v4 to mark the strings for translation or
> will you apply a fixup your end?

I didn't follow the _() discussion (was there any?)

I do not think lack of _() is a show-stopper and my preference is to
keep what I queued that does not have _(), and receive a separate
follow-up patch that changes "msg" to _("msg") and does nothing
else.

Thanks.

Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'

[Jeff King]

On Thu, Dec 15, 2016 at 09:36:17AM -0800, Junio C Hamano wrote:

> > Did you want me to send a v4 to mark the strings for translation or
> > will you apply a fixup your end?
> 
> I didn't follow the _() discussion (was there any?)

I think the discussion was just "we should do that".

> I do not think lack of _() is a show-stopper and my preference is to
> keep what I queued that does not have _(), and receive a separate
> follow-up patch that changes "msg" to _("msg") and does nothing
> else.

Here's a patch.

-- >8 --
Subject: merge: mark usage error strings for translation

The nearby error messages are already marked for
translation, but these new ones aren't.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/merge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 668aaffb8..599d25c4c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1164,7 +1164,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		const char *nargv[] = {"reset", "--merge", NULL};
 
 		if (orig_argc != 2)
-			usage_msg_opt("--abort expects no arguments",
+			usage_msg_opt(_("--abort expects no arguments"),
 			      builtin_merge_usage, builtin_merge_options);
 
 		if (!file_exists(git_path_merge_head()))
@@ -1180,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		const char *nargv[] = {"commit", NULL};
 
 		if (orig_argc != 2)
-			usage_msg_opt("--continue expects no arguments",
+			usage_msg_opt(_("--continue expects no arguments"),
 			      builtin_merge_usage, builtin_merge_options);
 
 		if (!file_exists(git_path_merge_head()))
-- 
2.11.0.348.g960a0b554