* Any interest in 'git merge --continue' as a command @ 2016-12-09 7:57 Chris Packham 2016-12-09 9:11 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Chris Packham @ 2016-12-09 7:57 UTC (permalink / raw) To: GIT 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-09 7:57 Any interest in 'git merge --continue' as a command Chris Packham @ 2016-12-09 9:11 ` Jeff King 2016-12-09 10:37 ` Jacob Keller 2016-12-09 19:16 ` Junio C Hamano 0 siblings, 2 replies; 26+ messages in thread From: Jeff King @ 2016-12-09 9:11 UTC (permalink / raw) To: Chris Packham; +Cc: GIT 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-09 9:11 ` Jeff King @ 2016-12-09 10:37 ` Jacob Keller 2016-12-09 19:16 ` Junio C Hamano 1 sibling, 0 replies; 26+ messages in thread From: Jacob Keller @ 2016-12-09 10:37 UTC (permalink / raw) To: Jeff King, Chris Packham; +Cc: GIT 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-09 9:11 ` Jeff King 2016-12-09 10:37 ` Jacob Keller @ 2016-12-09 19:16 ` Junio C Hamano 2016-12-10 8:49 ` Chris Packham 2016-12-10 8:59 ` Any interest in 'git merge --continue' as a command Jeff King 1 sibling, 2 replies; 26+ messages in thread From: Junio C Hamano @ 2016-12-09 19:16 UTC (permalink / raw) To: Jeff King; +Cc: Chris Packham, GIT 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-09 19:16 ` Junio C Hamano @ 2016-12-10 8:49 ` Chris Packham 2016-12-10 9:00 ` Jeff King 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham 2016-12-10 8:59 ` Any interest in 'git merge --continue' as a command Jeff King 1 sibling, 2 replies; 26+ messages in thread From: Chris Packham @ 2016-12-10 8:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, GIT 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-10 8:49 ` Chris Packham @ 2016-12-10 9:00 ` Jeff King 2016-12-10 10:58 ` Jacob Keller 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham 1 sibling, 1 reply; 26+ messages in thread From: Jeff King @ 2016-12-10 9:00 UTC (permalink / raw) To: Chris Packham; +Cc: Junio C Hamano, GIT 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-10 9:00 ` Jeff King @ 2016-12-10 10:58 ` Jacob Keller 0 siblings, 0 replies; 26+ messages in thread From: Jacob Keller @ 2016-12-10 10:58 UTC (permalink / raw) To: Jeff King; +Cc: Chris Packham, Junio C Hamano, GIT 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-10 8:49 ` Chris Packham 2016-12-10 9:00 ` Jeff King @ 2016-12-12 8:34 ` Chris Packham 2016-12-12 9:02 ` Markus Hitter ` (2 more replies) 1 sibling, 3 replies; 26+ messages in thread From: Chris Packham @ 2016-12-12 8:34 UTC (permalink / raw) To: git; +Cc: peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham @ 2016-12-12 9:02 ` Markus Hitter 2016-12-13 8:33 ` Chris Packham 2016-12-12 9:40 ` Jeff King 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham 2 siblings, 1 reply; 26+ messages in thread From: Markus Hitter @ 2016-12-12 9:02 UTC (permalink / raw) To: Chris Packham, git; +Cc: peff, jacob.keller, gitster 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-12 9:02 ` Markus Hitter @ 2016-12-13 8:33 ` Chris Packham 0 siblings, 0 replies; 26+ messages in thread From: Chris Packham @ 2016-12-13 8:33 UTC (permalink / raw) To: Markus Hitter; +Cc: GIT, Jeff King, Jacob Keller, Junio C Hamano 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham 2016-12-12 9:02 ` Markus Hitter @ 2016-12-12 9:40 ` Jeff King 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham 2 siblings, 0 replies; 26+ messages in thread From: Jeff King @ 2016-12-12 9:40 UTC (permalink / raw) To: Chris Packham; +Cc: git, jacob.keller, gitster 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham 2016-12-12 9:02 ` Markus Hitter 2016-12-12 9:40 ` Jeff King @ 2016-12-13 8:48 ` Chris Packham 2016-12-13 8:48 ` [PATCHv2 2/2] completion: add --continue option for merge Chris Packham ` (3 more replies) 2 siblings, 4 replies; 26+ messages in thread From: Chris Packham @ 2016-12-13 8:48 UTC (permalink / raw) To: git; +Cc: mah, peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCHv2 2/2] completion: add --continue option for merge 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham @ 2016-12-13 8:48 ` Chris Packham 2016-12-13 11:59 ` [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' Jeff King ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Chris Packham @ 2016-12-13 8:48 UTC (permalink / raw) To: git; +Cc: mah, peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham 2016-12-13 8:48 ` [PATCHv2 2/2] completion: add --continue option for merge Chris Packham @ 2016-12-13 11:59 ` Jeff King 2016-12-13 18:02 ` Junio C Hamano 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham 3 siblings, 0 replies; 26+ messages in thread From: Jeff King @ 2016-12-13 11:59 UTC (permalink / raw) To: Chris Packham; +Cc: git, mah, jacob.keller, gitster 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham 2016-12-13 8:48 ` [PATCHv2 2/2] completion: add --continue option for merge Chris Packham 2016-12-13 11:59 ` [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' Jeff King @ 2016-12-13 18:02 ` Junio C Hamano 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham 3 siblings, 0 replies; 26+ messages in thread From: Junio C Hamano @ 2016-12-13 18:02 UTC (permalink / raw) To: Chris Packham; +Cc: git, mah, peff, jacob.keller 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham ` (2 preceding siblings ...) 2016-12-13 18:02 ` Junio C Hamano @ 2016-12-14 8:37 ` Chris Packham 2016-12-14 8:37 ` [PATCH 2/3] completion: add --continue option for merge Chris Packham ` (3 more replies) 3 siblings, 4 replies; 26+ messages in thread From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw) To: git; +Cc: mah, peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/3] completion: add --continue option for merge 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham @ 2016-12-14 8:37 ` Chris Packham 2016-12-14 8:37 ` [PATCH 3/3] merge: Ensure '--abort' option takes no arguments Chris Packham ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw) To: git; +Cc: mah, peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/3] merge: Ensure '--abort' option takes no arguments 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham 2016-12-14 8:37 ` [PATCH 2/3] completion: add --continue option for merge Chris Packham @ 2016-12-14 8:37 ` Chris Packham 2016-12-14 15:20 ` [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' Jeff King 2016-12-14 18:04 ` Junio C Hamano 3 siblings, 0 replies; 26+ messages in thread From: Chris Packham @ 2016-12-14 8:37 UTC (permalink / raw) To: git; +Cc: mah, peff, jacob.keller, gitster, 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham 2016-12-14 8:37 ` [PATCH 2/3] completion: add --continue option for merge Chris Packham 2016-12-14 8:37 ` [PATCH 3/3] merge: Ensure '--abort' option takes no arguments Chris Packham @ 2016-12-14 15:20 ` Jeff King 2016-12-14 17:01 ` Junio C Hamano 2016-12-14 18:04 ` Junio C Hamano 3 siblings, 1 reply; 26+ messages in thread From: Jeff King @ 2016-12-14 15:20 UTC (permalink / raw) To: Chris Packham; +Cc: git, mah, jacob.keller, gitster 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-14 15:20 ` [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' Jeff King @ 2016-12-14 17:01 ` Junio C Hamano 0 siblings, 0 replies; 26+ messages in thread From: Junio C Hamano @ 2016-12-14 17:01 UTC (permalink / raw) To: Jeff King; +Cc: Chris Packham, git, mah, jacob.keller 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham ` (2 preceding siblings ...) 2016-12-14 15:20 ` [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' Jeff King @ 2016-12-14 18:04 ` Junio C Hamano 2016-12-15 7:29 ` Chris Packham 3 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2016-12-14 18:04 UTC (permalink / raw) To: Chris Packham; +Cc: git, mah, peff, jacob.keller 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-14 18:04 ` Junio C Hamano @ 2016-12-15 7:29 ` Chris Packham 2016-12-15 17:36 ` Junio C Hamano 0 siblings, 1 reply; 26+ messages in thread From: Chris Packham @ 2016-12-15 7:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: GIT, Markus Hitter, Jeff King, Jacob Keller 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-15 7:29 ` Chris Packham @ 2016-12-15 17:36 ` Junio C Hamano 2016-12-15 17:43 ` Jeff King 0 siblings, 1 reply; 26+ messages in thread From: Junio C Hamano @ 2016-12-15 17:36 UTC (permalink / raw) To: Chris Packham; +Cc: GIT, Markus Hitter, Jeff King, Jacob Keller 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' 2016-12-15 17:36 ` Junio C Hamano @ 2016-12-15 17:43 ` Jeff King 0 siblings, 0 replies; 26+ messages in thread From: Jeff King @ 2016-12-15 17:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Chris Packham, GIT, Markus Hitter, Jacob Keller 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-09 19:16 ` Junio C Hamano 2016-12-10 8:49 ` Chris Packham @ 2016-12-10 8:59 ` Jeff King 2016-12-10 18:16 ` Junio C Hamano 1 sibling, 1 reply; 26+ messages in thread From: Jeff King @ 2016-12-10 8:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: Chris Packham, GIT 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Any interest in 'git merge --continue' as a command 2016-12-10 8:59 ` Any interest in 'git merge --continue' as a command Jeff King @ 2016-12-10 18:16 ` Junio C Hamano 0 siblings, 0 replies; 26+ messages in thread From: Junio C Hamano @ 2016-12-10 18:16 UTC (permalink / raw) To: Jeff King; +Cc: Chris Packham, GIT 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2016-12-15 17:50 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-09 7:57 Any interest in 'git merge --continue' as a command Chris Packham 2016-12-09 9:11 ` Jeff King 2016-12-09 10:37 ` Jacob Keller 2016-12-09 19:16 ` Junio C Hamano 2016-12-10 8:49 ` Chris Packham 2016-12-10 9:00 ` Jeff King 2016-12-10 10:58 ` Jacob Keller 2016-12-12 8:34 ` [RFC/PATCH] merge: Add '--continue' option as a synonym for 'git commit' Chris Packham 2016-12-12 9:02 ` Markus Hitter 2016-12-13 8:33 ` Chris Packham 2016-12-12 9:40 ` Jeff King 2016-12-13 8:48 ` [PATCHv2 1/2] " Chris Packham 2016-12-13 8:48 ` [PATCHv2 2/2] completion: add --continue option for merge Chris Packham 2016-12-13 11:59 ` [PATCHv2 1/2] merge: Add '--continue' option as a synonym for 'git commit' Jeff King 2016-12-13 18:02 ` Junio C Hamano 2016-12-14 8:37 ` [PATCHv3 1/3] " Chris Packham 2016-12-14 8:37 ` [PATCH 2/3] completion: add --continue option for merge Chris Packham 2016-12-14 8:37 ` [PATCH 3/3] merge: Ensure '--abort' option takes no arguments Chris Packham 2016-12-14 15:20 ` [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit' Jeff King 2016-12-14 17:01 ` Junio C Hamano 2016-12-14 18:04 ` Junio C Hamano 2016-12-15 7:29 ` Chris Packham 2016-12-15 17:36 ` Junio C Hamano 2016-12-15 17:43 ` Jeff King 2016-12-10 8:59 ` Any interest in 'git merge --continue' as a command Jeff King 2016-12-10 18:16 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.