rcs_commit and rcs_commit_staged api changes

Using named parameters for these is overdue. Passing the session in a
parameter instead of passing username and IP separately will later allow
storing other session info, like username or part of the email.

Note that these functions are not part of the exported API,
and the prototype change will catch (most) skew, so I am not changing
API versions. Any third-party plugins that call them will need updated
though.

17 files changed, 262 insertions, 245 deletions

diff --git a/IkiWiki/Plugin/attachment.pm b/IkiWiki/Plugin/attachment.pm
index 216e00b3d..ee105a170 100644
--- a/IkiWiki/Plugin/attachment.pm
+++ b/IkiWiki/Plugin/attachment.pm
@@ -183,10 +183,12 @@ sub formbuilder (@) {
 		if ($config{rcs}) {
 			IkiWiki::rcs_add($filename);
 			IkiWiki::disable_commit_hook();
-			IkiWiki::rcs_commit($filename, gettext("attachment upload"),
-				IkiWiki::rcs_prepedit($filename),
-				$session->param("name"),
-				$session->remote_addr());
+			IkiWiki::rcs_commit(
+				file => $filename,
+				message => gettext("attachment upload"),
+				token => IkiWiki::rcs_prepedit($filename),
+				session => $session,
+			);
 			IkiWiki::enable_commit_hook();
 			IkiWiki::rcs_update();
 		}
diff --git a/IkiWiki/Plugin/autoindex.pm b/IkiWiki/Plugin/autoindex.pm
index c3eb53300..11595e217 100644
--- a/IkiWiki/Plugin/autoindex.pm
+++ b/IkiWiki/Plugin/autoindex.pm
@@ -117,8 +117,8 @@ sub refresh () {
 		}
 		if ($config{rcs}) {
 			IkiWiki::rcs_commit_staged(
-				gettext("automatic index generation"),
-				undef, undef);
+				message => gettext("automatic index generation"),
+			);
 			IkiWiki::enable_commit_hook();
 		}
 	}
diff --git a/IkiWiki/Plugin/bzr.pm b/IkiWiki/Plugin/bzr.pm
index 44ab9a86a..562d5d389 100644
--- a/IkiWiki/Plugin/bzr.pm
+++ b/IkiWiki/Plugin/bzr.pm
@@ -123,8 +123,13 @@ sub rcs_prepedit ($) {
 	return "";
 }
 
-sub bzr_author ($$) {
-	my ($user, $ipaddr) = @_;
+sub bzr_author ($) {
+	my $session=shift;
+
+	return unless defined $session;
+
+	my $user=$session->param("name");
+	my $ipaddr=$session->remote_addr();
 
 	if (defined $user) {
 		return IkiWiki::possibly_foolish_untaint($user);
@@ -137,18 +142,19 @@ sub bzr_author ($$) {
 	}
 }
 
-sub rcs_commit ($$$;$$$) {
-	my ($file, $message, $rcstoken, $user, $ipaddr, $emailuser) = @_;
+sub rcs_commit (@) {
+	my %params=@_;
 
-	$user = bzr_author($user, $ipaddr);
+	my $user=bzr_author($params{session});
 
-	$message = IkiWiki::possibly_foolish_untaint($message);
-	if (! length $message) {
-		$message = "no message given";
+	$params{message} = IkiWiki::possibly_foolish_untaint($params{message});
+	if (! length $params{message}) {
+		$params{message} = "no message given";
 	}
 
-	my @cmdline = ("bzr", "commit", "--quiet", "-m", $message, "--author", $user,
-	               $config{srcdir}."/".$file);
+	my @cmdline = ("bzr", "commit", "--quiet", "-m", $params{message},
+	               (defined $user ? ("--author", $user) : ()),
+	               $config{srcdir}."/".$params{file});
 	if (system(@cmdline) != 0) {
 		warn "'@cmdline' failed: $!";
 	}
@@ -156,19 +162,18 @@ sub rcs_commit ($$$;$$$) {
 	return undef; # success
 }
 
-sub rcs_commit_staged ($$$;$) {
-	# Commits all staged changes. Changes can be staged using rcs_add,
-	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
+sub rcs_commit_staged (@) {
+	my %params=@_;
 
-	$user = bzr_author($user, $ipaddr);
+	my $user=bzr_author($params{session});
 
-	$message = IkiWiki::possibly_foolish_untaint($message);
-	if (! length $message) {
-		$message = "no message given";
+	$params{message} = IkiWiki::possibly_foolish_untaint($params{message});
+	if (! length $params{message}) {
+		$params{message} = "no message given";
 	}
 
-	my @cmdline = ("bzr", "commit", "--quiet", "-m", $message, "--author", $user,
+	my @cmdline = ("bzr", "commit", "--quiet", "-m", $params{message},
+	               (defined $user ? ("--author", $user) : ()),
 	               $config{srcdir});
 	if (system(@cmdline) != 0) {
 		warn "'@cmdline' failed: $!";
diff --git a/IkiWiki/Plugin/comments.pm b/IkiWiki/Plugin/comments.pm
index 4770209c9..41c6948e8 100644
--- a/IkiWiki/Plugin/comments.pm
+++ b/IkiWiki/Plugin/comments.pm
@@ -513,9 +513,10 @@ sub editcomment ($$) {
 
 			IkiWiki::rcs_add($file);
 			IkiWiki::disable_commit_hook();
-			$conflict = IkiWiki::rcs_commit_staged($message,
-				$session->param('name'),
-				$session->remote_addr());
+			$conflict = IkiWiki::rcs_commit_staged(
+				message => $message,
+				session => $session,
+			);
 			IkiWiki::enable_commit_hook();
 			IkiWiki::rcs_update();
 		}
@@ -603,9 +604,10 @@ sub commentmoderation ($$) {
 			if ($config{rcs} and $config{comments_commit}) {
 				my $message = gettext("Comment moderation");
 				IkiWiki::disable_commit_hook();
-				$conflict=IkiWiki::rcs_commit_staged($message,
-					$session->param('name'),
-					$session->remote_addr());
+				$conflict=IkiWiki::rcs_commit_staged(
+					message => $message,
+					session => $session,
+				);
 				IkiWiki::enable_commit_hook();
 				IkiWiki::rcs_update();
 			}
diff --git a/IkiWiki/Plugin/cvs.pm b/IkiWiki/Plugin/cvs.pm
index a9fe162a1..c6687d780 100644
--- a/IkiWiki/Plugin/cvs.pm
+++ b/IkiWiki/Plugin/cvs.pm
@@ -183,41 +183,47 @@ sub rcs_prepedit ($) {
 	return defined $rev ? $rev : "";
 }
 
-sub rcs_commit ($$$;$$$) {
+sub commitmessage (@) {
+	my %params=@_;
+	
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			return "web commit by ".
+				$params{session}->param("name").
+				(length $params{message} ? ": $params{message}" : "");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			return "web commit from ".
+				$params{session}->remote_addr().
+				(length $params{message} ? ": $params{message}" : "");
+		}
+	}
+	return $params{message};
+}
+
+sub rcs_commit (@) {
 	# Tries to commit the page; returns undef on _success_ and
 	# a version of the page with the rcs's conflict markers on failure.
 	# The file is relative to the srcdir.
-	my $file=shift;
-	my $message=shift;
-	my $rcstoken=shift;
-	my $user=shift;
-	my $ipaddr=shift;
-	my $emailuser=shift;
+	my %params=@_;
 
 	return unless cvs_is_controlling;
 
-	if (defined $user) {
-		$message="web commit by $user".(length $message ? ": $message" : "");
-	}
-	elsif (defined $ipaddr) {
-		$message="web commit from $ipaddr".(length $message ? ": $message" : "");
-	}
-
 	# Check to see if the page has been changed by someone
 	# else since rcs_prepedit was called.
-	my ($oldrev)=$rcstoken=~/^([0-9]+)$/; # untaint
-	my $rev=cvs_info("Repository revision", "$config{srcdir}/$file");
+	my ($oldrev)=$params{token}=~/^([0-9]+)$/; # untaint
+	my $rev=cvs_info("Repository revision", "$config{srcdir}/$params{file}");
 	if (defined $rev && defined $oldrev && $rev != $oldrev) {
 		# Merge their changes into the file that we've
 		# changed.
-		cvs_runcvs('update', $file) ||
+		cvs_runcvs('update', $params{file}) ||
 			warn("cvs merge from $oldrev to $rev failed\n");
 	}
 
 	if (! cvs_runcvs('commit', '-m',
-			 IkiWiki::possibly_foolish_untaint $message)) {
-		my $conflict=readfile("$config{srcdir}/$file");
-		cvs_runcvs('update', '-C', $file) ||
+			 IkiWiki::possibly_foolish_untaint(commitmessage(%params)))) {
+		my $conflict=readfile("$config{srcdir}/$params{file}");
+		cvs_runcvs('update', '-C', $params{file}) ||
 			warn("cvs revert failed\n");
 		return $conflict;
 	}
@@ -225,20 +231,13 @@ sub rcs_commit ($$$;$$$) {
 	return undef # success
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
-
-	if (defined $user) {
-		$message="web commit by $user".(length $message ? ": $message" : "");
-	}
-	elsif (defined $ipaddr) {
-		$message="web commit from $ipaddr".(length $message ? ": $message" : "");
-	}
+	my %params=@_;
 
 	if (! cvs_runcvs('commit', '-m',
-			 IkiWiki::possibly_foolish_untaint $message)) {
+			 IkiWiki::possibly_foolish_untaint(commitmessage(%params)))) {
 		warn "cvs staged commit failed\n";
 		return 1; # failure
 	}
diff --git a/IkiWiki/Plugin/darcs.pm b/IkiWiki/Plugin/darcs.pm
index 345456c01..0dfc8708d 100644
--- a/IkiWiki/Plugin/darcs.pm
+++ b/IkiWiki/Plugin/darcs.pm
@@ -140,14 +140,31 @@ sub rcs_prepedit ($) {
 	return $rev;
 }
 
-sub rcs_commit ($$$;$$$) {
+sub commitauthor (@) {
+	my %params=@_;
+	
+	my $author="anon\@web";
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			return $params{session}->param("name").'@web';
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			return $params{session}->remote_addr().'@web';
+		}
+	}
+	return 'anon@web';
+}
+
+sub rcs_commit (@) {
 	# Commit the page.  Returns 'undef' on success and a version of the page
 	# with conflict markers on failure.
+	my %params=@_;
 
-	my ($file, $message, $rcstoken, $user, $ipaddr, $emailuser) = @_;
+	my ($file, $message, $token) =
+		($params{file}, $params{message}, $params{token});
 
 	# Compute if the "revision" of $file changed.
-	my $changed = darcs_rev($file) ne $rcstoken;
+	my $changed = darcs_rev($file) ne $token;
 
 	# Yes, the following is a bit convoluted.
 	if ($changed) {
@@ -155,7 +172,7 @@ sub rcs_commit ($$$;$$$) {
 		rename("$config{srcdir}/$file", "$config{srcdir}/$file.save") or
 			error("failed to rename $file to $file.save: $!");
 
-		# Roll the repository back to $rcstoken.
+		# Roll the repository back to $token.
 
 		# TODO.  Can we be sure that no changes are lost?  I think that
 		# we can, if we make sure that the 'darcs push' below will always
@@ -166,37 +183,28 @@ sub rcs_commit ($$$;$$$) {
 		# TODO: 'yes | ...' needed?  Doesn't seem so.
 		silentsystem('darcs', "revert", "--repodir", $config{srcdir}, "--all") == 0 ||
 			error("'darcs revert' failed");
-		# Remove all patches starting at $rcstoken.
+		# Remove all patches starting at $token.
 		my $child = open(DARCS_OBLITERATE, "|-");
 		if (! $child) {
 			open(STDOUT, ">/dev/null");
 			exec('darcs', "obliterate", "--repodir", $config{srcdir},
-			   "--match", "hash " . $rcstoken) and
+			   "--match", "hash " . $token) and
 			   error("'darcs obliterate' failed");
 		}
 		1 while print DARCS_OBLITERATE "y";
 		close(DARCS_OBLITERATE);
-		# Restore the $rcstoken one.
+		# Restore the $token one.
 		silentsystem('darcs', "pull", "--quiet", "--repodir", $config{srcdir},
-			"--match", "hash " . $rcstoken, "--all") == 0 ||
+			"--match", "hash " . $token, "--all") == 0 ||
 			error("'darcs pull' failed");
 	
-		# We're back at $rcstoken.  Re-install the modified file.
+		# We're back at $token.  Re-install the modified file.
 		rename("$config{srcdir}/$file.save", "$config{srcdir}/$file") or
 			error("failed to rename $file.save to $file: $!");
 	}
 
 	# Record the changes.
-	my $author;
-	if (defined $user) {
-		$author = "$user\@web";
-	}
-	elsif (defined $ipaddr) {
-		$author = "$ipaddr\@web";
-	}
-	else {
-		$author = "anon\@web";
-	}
+	my $author=commitauthor(%params);
 	if (!defined $message || !length($message)) {
 		$message = "empty message";
 	}
@@ -211,13 +219,13 @@ sub rcs_commit ($$$;$$$) {
 
 	# If this updating yields any conflicts, we'll record them now to resolve
 	# them.  If nothing is recorded, there are no conflicts.
-	$rcstoken = darcs_rev($file);
+	$token = darcs_rev($file);
 	# TODO: Use only the first line here, i.e. only the patch name?
 	writefile("$file.log", $config{srcdir}, 'resolve conflicts: ' . $message);
 	silentsystem('darcs', 'record', '--repodir', $config{srcdir}, '--all',
 		'-m', 'resolve conflicts: ' . $message, '--author', $author, $file) == 0 ||
 		error("'darcs record' failed");
-	my $conflicts = darcs_rev($file) ne $rcstoken;
+	my $conflicts = darcs_rev($file) ne $token;
 	unlink("$config{srcdir}/$file.log") or
 		error("failed to remove '$file.log'");
 
@@ -239,25 +247,18 @@ sub rcs_commit ($$$;$$$) {
 	}
 }
 
-sub rcs_commit_staged ($$$;$) {
-	my ($message, $user, $ipaddr, $emailuser) = @_;
+sub rcs_commit_staged (@) {
+	my %params=@_;
 
-	my $author;
-	if (defined $user) {
-		$author = "$user\@web";
-	}
-	elsif (defined $ipaddr) {
-		$author = "$ipaddr\@web";
-	}
-	else {
-		$author = "anon\@web";
-	}
-	if (!defined $message || !length($message)) {
-		$message = "empty message";
+	my $author=commitauthor(%params);
+	if (!defined $params{message} || !length($params{message})) {
+		$params{message} = "empty message";
 	}
 
-	silentsystem('darcs', "record", "--repodir", $config{srcdir}, "-a", "-A", $author,
-		"-m", $message)	== 0 || error("'darcs record' failed");
+	silentsystem('darcs', "record", "--repodir", $config{srcdir},
+		"-a", "-A", $author,
+		"-m", $params{message},
+	) == 0 || error("'darcs record' failed");
 
 	# Push the changes to the main repository.
 	silentsystem('darcs', 'push', '--quiet', '--repodir', $config{srcdir}, '--all') == 0 ||
diff --git a/IkiWiki/Plugin/editpage.pm b/IkiWiki/Plugin/editpage.pm
index a8e75121f..1a04a72b5 100644
--- a/IkiWiki/Plugin/editpage.pm
+++ b/IkiWiki/Plugin/editpage.pm
@@ -401,10 +401,12 @@ sub cgi_editpage ($$) {
 			# signaling to it that it should not try to
 			# do anything.
 			disable_commit_hook();
-			$conflict=rcs_commit($file, $message,
-				$form->field("rcsinfo"),
-				$session->param("name"),
-				$session->remote_addr());
+			$conflict=rcs_commit(
+				file => $file,
+				message => $message,
+				token => $form->field("rcsinfo"),
+				session => $session,
+			);
 			enable_commit_hook();
 			rcs_update();
 		}
diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm
index b56f229d7..b02fc118d 100644
--- a/IkiWiki/Plugin/git.pm
+++ b/IkiWiki/Plugin/git.pm
@@ -464,43 +464,55 @@ sub rcs_prepedit ($) {
 	return git_sha1($file);
 }
 
-sub rcs_commit ($$$;$$$) {
+sub rcs_commit (@) {
 	# Try to commit the page; returns undef on _success_ and
 	# a version of the page with the rcs's conflict markers on
 	# failure.
-
-	my ($file, $message, $rcstoken, $user, $ipaddr, $emailuser) = @_;
+	my %params=@_;
 
 	# Check to see if the page has been changed by someone else since
 	# rcs_prepedit was called.
-	my $cur    = git_sha1($file);
-	my ($prev) = $rcstoken =~ /^($sha1_pattern)$/; # untaint
+	my $cur    = git_sha1($params{file});
+	my ($prev) = $params{token} =~ /^($sha1_pattern)$/; # untaint
 
 	if (defined $cur && defined $prev && $cur ne $prev) {
-		my $conflict = merge_past($prev, $file, $dummy_commit_msg);
+		my $conflict = merge_past($prev, $params{file}, $dummy_commit_msg);
 		return $conflict if defined $conflict;
 	}
 
-	rcs_add($file);	
-	return rcs_commit_staged($message, $user, $ipaddr);
+	rcs_add($params{file});
+	return rcs_commit_staged(
+		message => $params{message},
+		session => $params{session},
+	);
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
-
-	# Set the commit author and email to the web committer.
+	my %params=@_;
+	
 	my %env=%ENV;
-	if (defined $user || defined $ipaddr) {
-		my $u=encode_utf8(defined $user ? $user : $ipaddr);
-		$ENV{GIT_AUTHOR_NAME}=$u;
-		$ENV{GIT_AUTHOR_EMAIL}="$u\@web";
+
+	if (defined $params{session}) {
+		# Set the commit author and email based on web session info.
+		my $u;
+		if (defined $params{session}->param("name")) {
+			$u=$params{session}->param("name");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			$u=$params{session}->remote_addr();
+		}
+		if (defined $u) {
+			$u=encode_utf8($u);
+			$ENV{GIT_AUTHOR_NAME}=$u;
+			$ENV{GIT_AUTHOR_EMAIL}="$u\@web";
+		}
 	}
 
-	$message = IkiWiki::possibly_foolish_untaint($message);
+	$params{message} = IkiWiki::possibly_foolish_untaint($params{message});
 	my @opts;
-	if ($message !~ /\S/) {
+	if ($params{message} !~ /\S/) {
 		# Force git to allow empty commit messages.
 		# (If this version of git supports it.)
 		my ($version)=`git --version` =~ /git version (.*)/;
@@ -508,13 +520,13 @@ sub rcs_commit_staged ($$$;$) {
 			push @opts, '--cleanup=verbatim';
 		}
 		else {
-			$message.=".";
+			$params{message}.=".";
 		}
 	}
 	push @opts, '-q';
 	# git commit returns non-zero if file has not been really changed.
 	# so we should ignore its exit status (hence run_or_non).
-	if (run_or_non('git', 'commit', @opts, '-m', $message)) {
+	if (run_or_non('git', 'commit', @opts, '-m', $params{message})) {
 		if (length $config{gitorigin_branch}) {
 			run_or_cry('git', 'push', $config{gitorigin_branch});
 		}
diff --git a/IkiWiki/Plugin/mercurial.pm b/IkiWiki/Plugin/mercurial.pm
index 1793ab4bb..edf915ae9 100644
--- a/IkiWiki/Plugin/mercurial.pm
+++ b/IkiWiki/Plugin/mercurial.pm
@@ -126,26 +126,26 @@ sub rcs_prepedit ($) {
 	return "";
 }
 
-sub rcs_commit ($$$;$$$) {
-	my ($file, $message, $rcstoken, $user, $ipaddr, $emailuser) = @_;
+sub rcs_commit (@) {
+	my %params=@_;
 
-	if (defined $user) {
-		$user = IkiWiki::possibly_foolish_untaint($user);
-	}
-	elsif (defined $ipaddr) {
-		$user = "Anonymous from ".IkiWiki::possibly_foolish_untaint($ipaddr);
-	}
-	else {
-		$user = "Anonymous";
+	my $user="Anonymous";
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			$user = $params{session}->param("name");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			$user = "Anonymous from ".$params{session}->remote_addr();
+		}
 	}
 
-	$message = IkiWiki::possibly_foolish_untaint($message);
-	if (! length $message) {
-		$message = "no message given";
+	if (! length $params{message}) {
+		$params{message} = "no message given";
 	}
 
 	my @cmdline = ("hg", "-q", "-R", $config{srcdir}, "commit", 
-	               "-m", $message, "-u", $user);
+	               "-m", IkiWiki::possibly_foolish_untaint($params{message}),
+	               "-u", IkiWiki::possibly_foolish_untaint($user));
 	if (system(@cmdline) != 0) {
 		warn "'@cmdline' failed: $!";
 	}
@@ -153,10 +153,10 @@ sub rcs_commit ($$$;$$$) {
 	return undef; # success
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
+	my %params=@_;
 	
 	error("rcs_commit_staged not implemented for mercurial"); # TODO
 }
diff --git a/IkiWiki/Plugin/monotone.pm b/IkiWiki/Plugin/monotone.pm
index 55d8039e0..95fbcee76 100644
--- a/IkiWiki/Plugin/monotone.pm
+++ b/IkiWiki/Plugin/monotone.pm
@@ -293,32 +293,33 @@ sub rcs_prepedit ($) {
 	return get_rev();
 }
 
-sub rcs_commit ($$$;$$$) {
+sub commitauthor (@) {
+	my %params=@_;
+
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			return "Web user: " . $params{session}->param("name");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			return "Web IP: " . $params{session}->remote_addr();
+		}
+	}
+	return "Web: Anonymous";
+}
+
+
+sub rcs_commit (@) {
 	# Tries to commit the page; returns undef on _success_ and
 	# a version of the page with the rcs's conflict markers on failure.
 	# The file is relative to the srcdir.
-	my $file=shift;
-	my $message=shift;
-	my $rcstoken=shift;
-	my $user=shift;
-	my $ipaddr=shift;
-	my $emailuser=shift;
-	my $author;
+	my %params=@_;
 
-	if (defined $user) {
-		$author="Web user: " . $user;
-	}
-	elsif (defined $ipaddr) {
-		$author="Web IP: " . $ipaddr;
-	}
-	else {
-		$author="Web: Anonymous";
-	}
+	my $author=IkiWiki::possibly_foolish_untaint(commitauthor(%params)),
 
 	chdir $config{srcdir}
 	    or error("Cannot chdir to $config{srcdir}: $!");
 
-	my ($oldrev)= $rcstoken=~ m/^($sha1_pattern)$/; # untaint
+	my ($oldrev) = $params{token} =~ m/^($sha1_pattern)$/; # untaint
 	my $rev = get_rev();
 	if (defined $rev && defined $oldrev && $rev ne $oldrev) {
 		my $automator = Monotone->new();
@@ -327,8 +328,8 @@ sub rcs_commit ($$$;$$$) {
 		# Something has been committed, has this file changed?
 		my ($out, $err);
 		$automator->setOpts("r", $oldrev, "r", $rev);
-		($out, $err) = $automator->call("content_diff", $file);
-		debug("Problem committing $file") if ($err ne "");
+		($out, $err) = $automator->call("content_diff", $params{file});
+		debug("Problem committing $params{file}") if ($err ne "");
 		my $diff = $out;
 		
 		if ($diff) {
@@ -337,11 +338,11 @@ sub rcs_commit ($$$;$$$) {
 			#
 			# first get the contents
 			debug("File changed: forming branch");
-			my $newfile=readfile("$config{srcdir}/$file");
+			my $newfile=readfile("$config{srcdir}/$params{file}");
 			
 			# then get the old content ID from the diff
-			if ($diff !~ m/^---\s$file\s+($sha1_pattern)$/m) {
-				error("Unable to find previous file ID for $file");
+			if ($diff !~ m/^---\s$params{file}\s+($sha1_pattern)$/m) {
+				error("Unable to find previous file ID for $params{file}");
 			}
 			my $oldFileID = $1;
 
@@ -352,13 +353,13 @@ sub rcs_commit ($$$;$$$) {
 			my $branch = $1;
 
 			# then put the new content into the DB (and record the new content ID)
-			my $newRevID = commit_file_to_new_rev($automator, $file, $oldFileID, $newfile, $oldrev, $branch, $author, $message);
+			my $newRevID = commit_file_to_new_rev($automator, $params{file}, $oldFileID, $newfile, $oldrev, $branch, $author, $params{message});
 
 			$automator->close();
 
 			# if we made it to here then the file has been committed... revert the local copy
-			if (system("mtn", "--root=$config{mtnrootdir}", "revert", $file) != 0) {
-				debug("Unable to revert $file after merge on conflicted commit!");
+			if (system("mtn", "--root=$config{mtnrootdir}", "revert", $params{file}) != 0) {
+				debug("Unable to revert $params{file} after merge on conflicted commit!");
 			}
 			debug("Divergence created! Attempting auto-merge.");
 
@@ -407,7 +408,7 @@ sub rcs_commit ($$$;$$$) {
 				# for cleanup note, this relies on the fact
 				# that ikiwiki seems to call rcs_prepedit()
 				# again after we return
-				return readfile("$config{srcdir}/$file");
+				return readfile("$config{srcdir}/$params{file}");
 			}
 			return undef;
 		}
@@ -419,11 +420,12 @@ sub rcs_commit ($$$;$$$) {
 
 	if (system("mtn", "--root=$config{mtnrootdir}", "commit", "--quiet",
 	           "--author", $author, "--key", $config{mtnkey}, "-m",
-		   IkiWiki::possibly_foolish_untaint($message), $file) != 0) {
+	           IkiWiki::possibly_foolish_untaint($params{message}),
+	           $params{file}) != 0) {
 		debug("Traditional commit failed! Returning data as conflict.");
-		my $conflict=readfile("$config{srcdir}/$file");
+		my $conflict=readfile("$config{srcdir}/$params{file}");
 		if (system("mtn", "--root=$config{mtnrootdir}", "revert",
-		           "--quiet", $file) != 0) {
+		           "--quiet", $params{file}) != 0) {
 			debug("monotone revert failed");
 		}
 		return $conflict;
@@ -439,32 +441,21 @@ sub rcs_commit ($$$;$$$) {
 	return undef # success
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
-	
+	my %params=@_;
+
 	# Note - this will also commit any spurious changes that happen to be
 	# lying around in the working copy.  There shouldn't be any, but...
 	
 	chdir $config{srcdir}
 	    or error("Cannot chdir to $config{srcdir}: $!");
 
-	my $author;
-
-	if (defined $user) {
-		$author="Web user: " . $user;
-	}
-	elsif (defined $ipaddr) {
-		$author="Web IP: " . $ipaddr;
-	}
-	else {
-		$author="Web: Anonymous";
-	}
-
 	if (system("mtn", "--root=$config{mtnrootdir}", "commit", "--quiet",
-	           "--author", $author, "--key", $config{mtnkey}, "-m",
-		   IkiWiki::possibly_foolish_untaint($message)) != 0) {
+	           "--author", IkiWiki::possibly_foolish_untaint(commitauthor(%params)),
+	           "--key", $config{mtnkey}, "-m",
+		   IkiWiki::possibly_foolish_untaint($params{message})) != 0) {
 		error("Monotone commit failed");
 	}
 }
diff --git a/IkiWiki/Plugin/norcs.pm b/IkiWiki/Plugin/norcs.pm
index 5131a1502..a3bb6240e 100644
--- a/IkiWiki/Plugin/norcs.pm
+++ b/IkiWiki/Plugin/norcs.pm
@@ -38,13 +38,11 @@ sub rcs_prepedit ($) {
 	return ""
 }
 
-sub rcs_commit ($$$;$$$) {
-	my ($file, $message, $rcstoken, $user, $ipaddr, $emailuser) = @_;
+sub rcs_commit (@) {
 	return undef # success
 }
 
-sub rcs_commit_staged ($$$;$) {
-	my ($message, $user, $ipaddr, $emailuser)=@_;
+sub rcs_commit_staged (@) {
 	return undef # success
 }
 
diff --git a/IkiWiki/Plugin/poll.pm b/IkiWiki/Plugin/poll.pm
index e50efa5e0..b333e2cdc 100644
--- a/IkiWiki/Plugin/poll.pm
+++ b/IkiWiki/Plugin/poll.pm
@@ -134,10 +134,12 @@ sub sessioncgi ($$) {
 		$oldchoice=$session->param($choice_param);
 		if ($config{rcs}) {
 			IkiWiki::disable_commit_hook();
-			IkiWiki::rcs_commit($pagesources{$page}, "poll vote ($choice)",
-				IkiWiki::rcs_prepedit($pagesources{$page}),
-				$session->param("name"),
-				$session->remote_addr());
+			IkiWiki::rcs_commit(
+				file => $pagesources{$page},
+				message => "poll vote ($choice)",
+				token => IkiWiki::rcs_prepedit($pagesources{$page}),
+				session => $session,
+			);
 			IkiWiki::enable_commit_hook();
 			IkiWiki::rcs_update();
 		}
diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
index b664cbf74..95f148183 100644
--- a/IkiWiki/Plugin/remove.pm
+++ b/IkiWiki/Plugin/remove.pm
@@ -213,9 +213,10 @@ sub sessioncgi ($$) {
 				foreach my $file (@files) {
 					IkiWiki::rcs_remove($file);
 				}
-				IkiWiki::rcs_commit_staged(gettext("removed"),
-					$session->param("name"),
-					$session->remote_addr());
+				IkiWiki::rcs_commit_staged(
+					message => gettext("removed"),
+					session => $session,
+				);
 				IkiWiki::enable_commit_hook();
 				IkiWiki::rcs_update();
 			}
diff --git a/IkiWiki/Plugin/rename.pm b/IkiWiki/Plugin/rename.pm
index 977c09e67..61d39d4b5 100644
--- a/IkiWiki/Plugin/rename.pm
+++ b/IkiWiki/Plugin/rename.pm
@@ -349,9 +349,8 @@ sub sessioncgi ($$) {
 				$pagesources{$rename->{src}}=$rename->{destfile};
 			}
 			IkiWiki::rcs_commit_staged(
-				sprintf(gettext("rename %s to %s"), $srcfile, $destfile),
-				$session->param("name"),
-				$session->remote_addr(),
+				message => sprintf(gettext("rename %s to %s"), $srcfile, $destfile),
+				session => $session,
 			) if $config{rcs};
 
 			# Then link fixups.
diff --git a/IkiWiki/Plugin/svn.pm b/IkiWiki/Plugin/svn.pm
index ffacb8cf9..f1e608408 100644
--- a/IkiWiki/Plugin/svn.pm
+++ b/IkiWiki/Plugin/svn.pm
@@ -144,44 +144,50 @@ sub rcs_prepedit ($) {
 	}
 }
 
-sub rcs_commit ($$$;$$$) {
+sub commitmessage (@) {
+	my %params=@_;
+
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			return "web commit by ".
+				$params{session}->param("name").
+				(length $params{message} ? ": $params{message}" : "");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			return "web commit from ".
+				$params{session}->remote_addr().
+				(length $params{message} ? ": $params{message}" : "");
+		}
+	}
+	return $params{message};
+}
+
+sub rcs_commit (@) {
 	# Tries to commit the page; returns undef on _success_ and
 	# a version of the page with the rcs's conflict markers on failure.
 	# The file is relative to the srcdir.
-	my $file=shift;
-	my $message=shift;
-	my $rcstoken=shift;
-	my $user=shift;
-	my $ipaddr=shift;
-	my $emailuser=shift;
-
-	if (defined $user) {
-		$message="web commit by $user".(length $message ? ": $message" : "");
-	}
-	elsif (defined $ipaddr) {
-		$message="web commit from $ipaddr".(length $message ? ": $message" : "");
-	}
+	my %params=@_;
 
 	if (-d "$config{srcdir}/.svn") {
 		# Check to see if the page has been changed by someone
 		# else since rcs_prepedit was called.
-		my ($oldrev)=$rcstoken=~/^([0-9]+)$/; # untaint
-		my $rev=svn_info("Revision", "$config{srcdir}/$file");
+		my ($oldrev)=$params{token}=~/^([0-9]+)$/; # untaint
+		my $rev=svn_info("Revision", "$config{srcdir}/$params{file}");
 		if (defined $rev && defined $oldrev && $rev != $oldrev) {
 			# Merge their changes into the file that we've
 			# changed.
 			if (system("svn", "merge", "--quiet", "-r$oldrev:$rev",
-			           "$config{srcdir}/$file", "$config{srcdir}/$file") != 0) {
+			           "$config{srcdir}/$params{file}", "$config{srcdir}/$params{file}") != 0) {
 				warn("svn merge -r$oldrev:$rev failed\n");
 			}
 		}
 
 		if (system("svn", "commit", "--quiet", 
 		           "--encoding", "UTF-8", "-m",
-		           IkiWiki::possibly_foolish_untaint($message),
+		           IkiWiki::possibly_foolish_untaint(commitmessage(%params)),
 			   $config{srcdir}) != 0) {
-			my $conflict=readfile("$config{srcdir}/$file");
-			if (system("svn", "revert", "--quiet", "$config{srcdir}/$file") != 0) {
+			my $conflict=readfile("$config{srcdir}/$params{file}");
+			if (system("svn", "revert", "--quiet", "$config{srcdir}/$params{file}") != 0) {
 				warn("svn revert failed\n");
 			}
 			return $conflict;
@@ -190,21 +196,14 @@ sub rcs_commit ($$$;$$$) {
 	return undef # success
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
-	
-	if (defined $user) {
-		$message="web commit by $user".(length $message ? ": $message" : "");
-	}
-	elsif (defined $ipaddr) {
-		$message="web commit from $ipaddr".(length $message ? ": $message" : "");
-	}
+	my %params=@_;
 	
 	if (system("svn", "commit", "--quiet",
 	           "--encoding", "UTF-8", "-m",
-	           IkiWiki::possibly_foolish_untaint($message),
+	           IkiWiki::possibly_foolish_untaint(commitmessage(%params)),
 		   $config{srcdir}) != 0) {
 		warn("svn commit failed\n");
 		return 1; # failure	
diff --git a/IkiWiki/Plugin/tag.pm b/IkiWiki/Plugin/tag.pm
index 62f030f4e..55064a9a3 100644
--- a/IkiWiki/Plugin/tag.pm
+++ b/IkiWiki/Plugin/tag.pm
@@ -90,7 +90,7 @@ sub gentag ($) {
 			if ($config{rcs}) {
 				IkiWiki::disable_commit_hook();
 				IkiWiki::rcs_add($tagfile);
-				IkiWiki::rcs_commit_staged($message, undef, undef);
+				IkiWiki::rcs_commit_staged(message => $message);
 				IkiWiki::enable_commit_hook();
 			}
 		});
diff --git a/IkiWiki/Plugin/tla.pm b/IkiWiki/Plugin/tla.pm
index 80c015e3c..da4385446 100644
--- a/IkiWiki/Plugin/tla.pm
+++ b/IkiWiki/Plugin/tla.pm
@@ -98,19 +98,23 @@ sub rcs_prepedit ($) {
 	}
 }
 
-sub rcs_commit ($$$;$$$) {
-	my $file=shift;
-	my $message=shift;
-	my $rcstoken=shift;
-	my $user=shift;
-	my $ipaddr=shift;
-	my $emailuser=shift;
-
-	if (defined $user) {
-		$message="web commit by $user".(length $message ? ": $message" : "");
-	}
-	elsif (defined $ipaddr) {
-		$message="web commit from $ipaddr".(length $message ? ": $message" : "");
+sub rcs_commit (@) {
+	my %params=@_;
+
+	my ($file, $message, $rcstoken)=
+		($params{file}, $params{message}, $params{token});
+
+	if (defined $params{session}) {
+		if (defined $params{session}->param("name")) {
+			$message="web commit by ".
+				$params{session}->param("name").
+				(length $message ? ": $message" : "");
+		}
+		elsif (defined $params{session}->remote_addr()) {
+			$message="web commit from ".
+				$params{session}->remote_addr().
+				(length $message ? ": $message" : "");
+		}
 	}
 
 	if (-d "$config{srcdir}/{arch}") {
@@ -140,10 +144,10 @@ sub rcs_commit ($$$;$$$) {
 	return undef # success
 }
 
-sub rcs_commit_staged ($$$;$) {
+sub rcs_commit_staged (@) {
 	# Commits all staged changes. Changes can be staged using rcs_add,
 	# rcs_remove, and rcs_rename.
-	my ($message, $user, $ipaddr, $emailuser)=@_;
+	my %params=@_;
 	
 	error("rcs_commit_staged not implemented for tla"); # TODO
 }