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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 26, 2011, 16:11 UTC
Message-ID: <20110526161102.GC24931@elie>
In-Reply-To: <1306425233-504-9-git-send-email-artagnon@gmail.com>

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)

[...]

Show 13 quoted lines

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