Subject: Re: [PATCH 08/10] revert: Introduce HEAD, TODO files to persist state, plan

From: Jonathan Nieder <[email-masked]>
Date: Thu, 26 May 2011 11:11:02 -0500

Ramkumar Ramachandra wrote:

> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -573,22 +580,86 @@ static void read_and_refresh_cache(const char *me, struct replay_opts *opts)
[...]
>  	prepare_revs(&revs, opts);
> +	persist_head(head);
> +	persist_todo(revs.commits, opts);
>  
>  	while ((commit = get_revision(&revs))) {
>  		int res = do_pick_commit(commit, opts);
> -		if (res)
> +		if (res) {
> +			commit_list_insert(commit, &revs.commits);
> +			persist_todo(revs.commits, opts);
>  			return res;
> +		}
>  	}

Almost there.  To comfort overly-worried people like me that think we
have not finished converted all die() calls yet, wouldn't this need to
look like

	persist_head(head);
	while ((commit = ...)) {
		int status_or_error;

		/*
		 * Checkpoint.  If do_pick_commit exits, make sure the user
		 * can still use "git cherry-pick --continue" to recover.
		 */
		persist_todo(revs.commits, opts);

		status_or_error = do_pick_commit(...);
		if (status_or_error)
			return status_or_error;
	}

	/* Success! */
	remove_todo(opts);
	remove_head();
	return 0;

And with that, this would no longer depend on the (valuable enough on
their own terms) patches 1 and 2 so they could be treated as a
separate series, no?