From 238e8b95a5b084e7131d3314b78419b9404cba4c Mon Sep 17 00:00:00 2001
From: Joey Hess <joey@kitenet.net>
Date: Wed, 6 Oct 2010 15:08:12 -0400
Subject: [PATCH] convert rcs_revert to only stage the reversion

---
 IkiWiki.pm                      |  4 ++--
 IkiWiki/Plugin/git.pm           | 13 ++++---------
 IkiWiki/Plugin/recentchanges.pm | 13 +++++++++----
 doc/plugins/write.mdwn          | 10 ++++------
 doc/todo/web_reversion.mdwn     |  4 ----
 5 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/IkiWiki.pm b/IkiWiki.pm
index 269647eb4..faf4af5c7 100644
--- a/IkiWiki.pm
+++ b/IkiWiki.pm
@@ -1941,11 +1941,11 @@ sub rcs_receive () {
 	$hooks{rcs}{rcs_receive}{call}->();
 }
 
-sub rcs_preprevert (@) {
+sub rcs_preprevert ($) {
 	$hooks{rcs}{rcs_preprevert}{call}->(@_);
 }
 
-sub rcs_revert (@) {
+sub rcs_revert ($) {
 	$hooks{rcs}{rcs_revert}{call}->(@_);
 }
 
diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
index 3ccaa446a..996ab6fba 100644
--- a/IkiWiki/Plugin/git.pm
+++ b/IkiWiki/Plugin/git.pm
@@ -846,19 +846,14 @@ sub rcs_preprevert (@) {
 		[git_parse_changes(git_commit_info($rev, 1))]);
 }
 
-sub rcs_revert (@) {
-	# Try to revert the given patch; returns undef on _success_.
-	my %params = @_;
-	my $rev = $params{rev};
+sub rcs_revert ($) {
+	# Try to revert the given rev; returns undef on _success_.
+	my $rev = $shift;
 
 	if (run_or_non('git', 'revert', '--no-commit', $rev)) {
-		debug "Committing revert for patch '$rev'.";
-		rcs_commit_staged(message => 
-			sprintf(gettext("This reverts commit %s"), $rev), @_);
+		return undef;
 	}
 	else {
-		# No idea what is actually getting reverted, so all we can
-		# do is say we failed.
 		run_or_die('git', 'reset', '--hard');
 		return sprintf(gettext("Failed to revert commit %s"), $rev);
 	}
diff --git a/IkiWiki/Plugin/recentchanges.pm b/IkiWiki/Plugin/recentchanges.pm
index 439241b93..44c981548 100644
--- a/IkiWiki/Plugin/recentchanges.pm
+++ b/IkiWiki/Plugin/recentchanges.pm
@@ -101,12 +101,17 @@ sub sessioncgi ($$) {
 	if ($form->submitted eq 'Revert' && $form->validate) {
 		IkiWiki::checksessionexpiry($q, $session, $q->param('sid'));
 		IkiWiki::disable_commit_hook();
-		my $r = IkiWiki::rcs_revert(
-			session => $session,
-			rev => $rev);
+		my $r = IkiWiki::rcs_revert($rev);
+		if (! defined $r) { # success
+			rcs_commit_staged(
+				message => sprintf(gettext("This reverts commit %s"), $rev),
+				session => $session,
+				rev => $rev,
+			);
+		}
 		IkiWiki::enable_commit_hook();
 	
-		if ($r) {
+		if (defined $r) {
 			die "Revert '$rev' failed.";
 		}
 		else {
diff --git a/doc/plugins/write.mdwn b/doc/plugins/write.mdwn
index 3eade34ee..dbbe83851 100644
--- a/doc/plugins/write.mdwn
+++ b/doc/plugins/write.mdwn
@@ -1201,16 +1201,14 @@ Like `rcs_receive`, it should do whatever sanity checks are appropriate
 for the RCS to limit changes to safe changes, and die if a change would
 be unsafe to revert.
 
-#### `rcs_revert(@)`
+#### `rcs_revert($)`
 
 This is called by the revert web interface. It is passed a named
 parameter rev that is the RCS-specific change ID to revert.
 
-Addition named parameters: `message`, and `session` (optional).
-
-It should try to revert the specified rev, which includes committing
-the reversion, and returns undef on _success_ and an error message
-on failure.
+It should try to revert the specified rev, and leave the reversion staged
+so `rcs_commit_staged` will complete it. It should return undef on _success_
+and an error message on failure.
 
 ### PageSpec plugins
 
diff --git a/doc/todo/web_reversion.mdwn b/doc/todo/web_reversion.mdwn
index 7cb412f79..784b72e05 100644
--- a/doc/todo/web_reversion.mdwn
+++ b/doc/todo/web_reversion.mdwn
@@ -71,10 +71,6 @@ Peter Gammie has done an initial implementation of the above.
 >>> to: --[[Joey]] 
 >>>> Please change the git pointer above, then. I will work on your branch. -- [[peteg]]
 
->>> * Would it be better for `rcs_revert` to not commit, and
->>>   `rcs_commit_staged` to then be used? This would work for git, but
->>>   maybe other RCSs would be problimatic. It would simplifiy the
->>>   interface and allow for future mulitple-revert interfaces.
 >>> * I quite don't understand why one caller of `git_parse_changes`
 >>>   needs it to chdir, and not the other one. It's running
 >>>   in the same git repo either way, and git doesn't need
-- 
2.44.0