Re: [PATCH] revert: add --stdin option to read commits from stdin

[Jonathan Nieder <jrnieder@gmail.com>]

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.

> --- 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.

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

 Documentation/git-cherry-pick.txt   |    7 ++++++
 builtin/revert.c                    |   37 ++++++++++++++++++-----------------
 t/t3501-revert-cherry-pick.sh       |   18 +++++++++++++++++
 t/t3508-cherry-pick-many-commits.sh |   24 ++++++++++++++++------
 4 files changed, 61 insertions(+), 25 deletions(-)