On Monday 14 June 2010 07:20:27 Jonathan Nieder wrote: > Christian Couder wrote: > > --- a/t/t3508-cherry-pick-many-commits.sh > > +++ b/t/t3508-cherry-pick-many-commits.sh > > @@ -92,4 +92,14 @@ test_expect_failure 'cherry-pick -3 fourth works' ' > > test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify > > fourth)" ' > > > > +test_expect_success 'cherry-pick --stdin works' ' > > + git checkout master && > > + git reset --hard first && > > [...] > > This test fails for me as written, since the previous test leaves some > files in an unmerged state. Patch 1 below works around that. Ok. > > --- a/builtin/revert.c > > +++ b/builtin/revert.c > > @@ -79,7 +80,7 @@ static void parse_args(int argc, const char **argv) > > } > > > > commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0); > > [...] > > > @@ -527,10 +528,12 @@ static void prepare_revs(struct rev_info *revs) > > { > > int argc = 0; > > int i; > > - const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv)); > > + const char **argv = xmalloc((commit_argc + 5) * sizeof(*argv)); > > > > argv[argc++] = NULL; > > argv[argc++] = "--no-walk"; > > + if (read_stdin) > > + argv[argc++] = "--stdin"; > > Ah, I see the problem now. But it would be even nicer to allow arbitrary > rev-list options, so a person could ‘git cherry-pick --reverse a..b’, > for example. Yeah, I agree that is nicer. > In other words, how about something like patch 2 below? > > Patch 3 is a small cleanup, as a bonus. > > Christian Couder (1): > revert: accept arbitrary rev-list options > > Jonathan Nieder (2): > t3508 (cherry-pick): futureproof against unmerged files > revert: do not rebuild argv on heap I get "indent with spaces" errors when I apply patches 2/3 and 3/3: $ git am ../messages/\[PATCH\ 2_3\]\ revert_\ accept\ arbitrary\ rev-list\ options.mbox Applying: revert: accept arbitrary rev-list options /home/christian/work/git/.git/rebase-apply/patch:35: indent with spaces. PARSE_OPT_KEEP_UNKNOWN); $ git am ../messages/\[PATCH\ 3_3\]\ revert_\ do\ not\ rebuild\ argv\ on\ heap.mbox Applying: revert: do not rebuild argv on heap /home/christian/work/git/.git/rebase-apply/patch:33: indent with spaces. PARSE_OPT_KEEP_ARGV0 | warning: 1 line adds whitespace errors. But otherwise it all looks very good to me. Even something like "git cherry-pick -3 fourth" is now working after patch 2/3, so all test cases now pass. This is because "--no-walk" does not always take over "-3" it looks like it depends on the order of the arguments. For example I get: $ git rev-list --no-walk -3 --reverse fourth 453a04748224b3f212580d1195b452334d346e75 e85abe28a2b8ef771f760575b325f4c41f9c815f 94d3184b3f0dcfebb393faf5a122dc429d775538 but: $ git rev-list -3 --no-walk --reverse fourth 94d3184b3f0dcfebb393faf5a122dc429d775538 I will post an updated v2 series without the whitespace errors and with a few documentation and test updates in patch 2/3. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html