X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/c87c7edf288ca69e96721ed6ff46f12b7f478540..0416a79dfe63d1484fc08c29847f639e1970216c:/doc/patchqueue/move_page.mdwn
diff --git a/doc/patchqueue/move_page.mdwn b/doc/patchqueue/move_page.mdwn
index fcf416a4a..21be9ba6b 100644
--- a/doc/patchqueue/move_page.mdwn
+++ b/doc/patchqueue/move_page.mdwn
@@ -1,25 +1,25 @@
-This is my first cut at a feature like that requested in [[todo/Moving_Pages]]. In case it gets mangled you can find it on [my site](http://www.betacantrips.com//attic/move.patch).
+This is my second cut at a feature like that requested in [[todo/Moving_Pages]].
+It can also be found [here](http://ikidev.betacantrips.com/patches/move.patch).
-A bunch of obvious shortcomings exist:
+A few shortcomings exist:
-* I'm not sure all the untaints are safe.
* No precautions whatsoever are made to protect against race conditions or failures
- in the rcs\_move function.
-* movepage.tmpl doesn't exist yet.
+ in the rcs\_move function. I didn't even do the `cgi_editpage` thing where I hold
+ the lock and render afterwards (mostly because the copy I was editing was not
+ up-to-date enough to have that code). Although FAILED_SAVE is in movepage.tmpl,
+ no code activates it yet.
* Some code is duplicated between cgi\_movepage and cgi\_editpage, as well
as rcs\_commit and rcs\_move.
-* The user interface is pretty lame -- there's no handy select list full
- of possible places to move it or anything.
-* I don't think I implemented cancelling.
-* from is redundant with page.
-* I don't think I called the right hook functions.
+* The user interface is pretty lame. I couldn't figure out a good way to let
+ the user specify which directory to move things to without implementing a
+ FileChooser thing.
* No redirect pages like those mentioned on [[todo/Moving_Pages]] exist yet,
so none are created.
-* It's not possible to get there through the actions listed on the wiki page.
- Instead you can select "Edit" and then change "edit" to "move" in the
- location bar.
-
-Anyhow, here's the patch, for whatever good it does.
+* I added a Move link to page.tmpl but it may belong better someplace else --
+ maybe editpage.tmpl? Not sure.
+* from is redundant with page so far -- but since the Move links could someday
+ come from someplace other than the page itself I kept it around.
+* If I move foo.mdwn to bar.mdwn, foo/* should move too, probably.
> Looks like a good start, although I agree about many of the points above,
> and also feel that something needs to be done about rcses that don't
@@ -29,165 +29,246 @@ Anyhow, here's the patch, for whatever good it does.
Hmm. Shouldn't that be done on a by-RCS basis, though? (i.e. implemented
by backends in the `rcs_move` function)
+> Probably, yes, but maybe there's a way to avoid duplicating code for that
+> in several of them.
+
Also, how should ikiwiki react if a page is edited (say, by another user)
before it is moved? Bail, or shrug and proceed?
-Could you elaborate on [[commit-internals]]? Can I assume that ikiwiki's
-working copy W will always reflect a revision of the master copy M?
-(That is, nobody changes W and leaves it uncommitted.) I would guess
-probably not; a user probably expects that if he starts editing W it
-won't get randomly committed by web user actions. But then looking at
-the svn backend, it looks like if I edit foo.mdwn, don't commit, and then
-a web user makes different changes, my changes get wiped out. So does
-W "belong" to ikiwiki? --Ethan
-
+> The important thing is to keep in mind that the page could be edited,
+> moved, deleted, etc in between the user starting the move and the move
+> happening. So, the code really needs to deal with all of these cases in
+> some way. It seems fine to me to go ahead with the move even if the page
+> was edited. If the page was deleted or moved, it seems reasonable to exit
+> with an error.
- diff -urx .svn ikiwiki/IkiWiki/CGI.pm ikiwiki-new/IkiWiki/CGI.pm
- --- ikiwiki/IkiWiki/CGI.pm 2007-01-04 03:52:47.000000000 -0800
- +++ ikiwiki-new/IkiWiki/CGI.pm 2007-01-11 18:49:37.000000000 -0800
- @@ -523,6 +523,97 @@
- }
+ diff -urNX ignorepats ikiwiki/IkiWiki/CGI.pm ikidev/IkiWiki/CGI.pm
+ --- ikiwiki/IkiWiki/CGI.pm 2007-02-14 18:17:12.000000000 -0800
+ +++ ikidev/IkiWiki/CGI.pm 2007-02-22 18:54:23.194982000 -0800
+ @@ -561,6 +561,106 @@
+ }
} #}}}
-
+
+sub cgi_movepage($$) {
- + my $q = shift;
- + my $session = shift;
- + eval q{use CGI::FormBuilder};
- + error($@) if $@;
- + my @fields=qw(do from rcsinfo subpage page newname message); # subpage ignored so far
- + my @buttons=("Rename Page", "Cancel");
+ + my $q = shift;
+ + my $session = shift;
+ + eval q{use CGI::FormBuilder};
+ + error($@) if $@;
+ + my @fields=qw(do from rcsinfo page newdir newname comments);
+ + my @buttons=("Rename Page", "Cancel");
+
- + my $form = CGI::FormBuilder->new(
- + fields => \@fields,
+ + my $form = CGI::FormBuilder->new(
+ + fields => \@fields,
+ header => 1,
+ charset => "utf-8",
+ method => 'POST',
- + action => $config{cgiurl},
+ + action => $config{cgiurl},
+ template => (-e "$config{templatedir}/movepage.tmpl" ?
- + {template_params("movepage.tpml")} : ""),
- + );
- + run_hooks(formbuilder_setup => sub {
- + shift->(form => $form, cgi => $q, session => $session);
- + });
+ + {template_params("movepage.tmpl")} : ""),
+ + );
+ + run_hooks(formbuilder_setup => sub {
+ + shift->(form => $form, cgi => $q, session => $session);
+ + });
+ +
+ + decode_form_utf8($form);
+
- + decode_form_utf8($form);
- +
- + # This untaint is safe because if the page doesn't exist, bail.
- + my $page = $form->field('page');
- + $page = possibly_foolish_untaint($page);
- + if (! exists $pagesources{$page}) {
- + error("page does not exist");
- + }
- + my $file=$pagesources{$page};
- + my $type=pagetype($file);
+ + # This untaint is safe because if the page doesn't exist, bail.
+ + my $page = $form->field('page');
+ + $page = possibly_foolish_untaint($page);
+ + if (! exists $pagesources{$page}) {
+ + error("page does not exist");
+ + }
+ + my $file=$pagesources{$page};
+ + my $type=pagetype($file);
+
- + my $from;
- + if (defined $form->field('from')) {
- + ($from)=$form->field('from')=~/$config{wiki_file_regexp}/;
- + }
- +
- + $form->field(name => "do", type => 'hidden');
- + $form->field(name => "from", type => 'hidden');
- + $form->field(name => "rcsinfo", type => 'hidden');
- + $form->field(name => "subpage", type => 'hidden');
- + $form->field(name => "page", value => $page, force => 1);
- + $form->field(name => "newname", type => "text", size => 80);
- + $form->field(name => "message", type => "text", size => 80);
+ + my $from;
+ + if (defined $form->field('from')) {
+ + ($from)=$form->field('from')=~/$config{wiki_file_regexp}/;
+ + }
+
- + if (! $form->submitted) {
- + $form->field(name => "rcsinfo", value => rcs_prepedit($file),
- + force => 1);
- + }
+ + $form->field(name => "do", type => 'hidden');
+ + $form->field(name => "from", type => 'hidden');
+ + $form->field(name => "rcsinfo", type => 'hidden');
+ + $form->field(name => "newdir", type => 'text', size => 80);
+ + $form->field(name => "page", value => $page, force => 1);
+ + $form->field(name => "newname", type => "text", size => 80);
+ + $form->field(name => "comments", type => "text", size => 80);
+ + $form->tmpl_param("can_commit", $config{rcs});
+ + $form->tmpl_param("indexlink", indexlink());
+ + $form->tmpl_param("baseurl", baseurl());
+
- + if ($form->submitted eq "Cancel") {
- + redirect($q, "$config{url}/".htmlpage($from));
- + return;
- + }
- +
- + if (! $form->submitted || $form->submitted eq "Preview" ||
- + ! $form->validate) {
- + if ($form->field("do") eq "move"){
- + page_locked($page, $session);
- + $form->tmpl_param("page_select", 0);
- + $form->field(name => "page", type => 'hidden');
- + $form->field(name => "type", type => 'hidden');
- + $form->title(sprintf(gettext("moving %s"), pagetitle($page)));
- + if (! defined $form->field('newname') ||
- + ! length $form->field('newname')) {
- + $form->field(name => "newname",
- + value => pagetitle($page), force => 1);
- + }
+ + if (! $form->submitted) {
+ + $form->field(name => "rcsinfo", value => rcs_prepedit($file),
+ + force => 1);
+ + }
+
- + }
- + print $form->render(submit => \@buttons);
- + }
- + else{
- + # This untaint is safe because titlepage removes any problematic
- + # characters.
- + my ($newname)=$form->field('newname');
- + $newname=titlepage(possibly_foolish_untaint($newname));
- + if (! defined $newname || ! length $newname || file_pruned($newname, $config{srcdir}) || $newname=~/^\//) {
- + error("bad page name");
- + }
- + page_locked($page, $session);
+ + if ($form->submitted eq "Cancel") {
+ + redirect($q, "$config{url}/".htmlpage($page));
+ + return;
+ + }
+
- + my $newfile = $newname . ".$type";
- + my $message = $form->field('message');
- + unlockwiki();
- + rcs_move($file, $newfile, $message, $form->field("rcsinfo"),
- + $session->param("name"), $ENV{REMOTE_ADDR});
- + redirect($q, "$config{url}/".htmlpage($newname));
- + }
+ + if (! $form->submitted || ! $form->validate) {
+ + check_canedit($page, $q, $session);
+ + $form->tmpl_param("page_select", 0);
+ + $form->field(name => "page", type => 'hidden');
+ + $form->field(name => "type", type => 'hidden');
+ + $form->title(sprintf(gettext("moving %s"), pagetitle($page)));
+ + my $pname = basename($page);
+ + my $dname = dirname($page);
+ + if (! defined $form->field('newname') ||
+ + ! length $form->field('newname')) {
+ + $form->field(name => "newname",
+ + value => pagetitle($pname, 1), force => 1);
+ + }
+ + if (! defined $form->field('newdir') ||
+ + ! length $form->field('newdir')) {
+ + $form->field(name => "newdir",
+ + value => pagetitle($dname, 1), force => 1);
+ + }
+ + print $form->render(submit => \@buttons);
+ + }
+ + else{
+ + # This untaint is safe because titlepage removes any problematic
+ + # characters.
+ + my ($newname)=$form->field('newname');
+ + $newname=titlepage(possibly_foolish_untaint($newname));
+ + my ($newdir)=$form->field('newdir');
+ + $newdir=titlepage(possibly_foolish_untaint($newdir));
+ + if (! defined $newname || ! length $newname || file_pruned($newname, $config{srcdir}) || $newname=~/^\//) {
+ + error("bad page name");
+ + }
+ + check_canedit($page, $q, $session);
+ +
+ + my $newpage = ($newdir?"$newdir/":"") . $newname;
+ + my $newfile = $newpage . ".$type";
+ + my $message = $form->field('comments');
+ + unlockwiki();
+ + rcs_move($file, $newfile, $message, $form->field("rcsinfo"),
+ + $session->param("name"), $ENV{REMOTE_ADDR});
+ + redirect($q, "$config{url}/".htmlpage($newpage));
+ + }
+}
+
sub cgi_getsession ($) { #{{{
- my $q=shift;
-
- @@ -631,6 +722,9 @@
- if ($do eq 'create' || $do eq 'edit') {
- cgi_editpage($q, $session);
- }
- + elsif ($do eq 'move') {
- + cgi_movepage($q, $session);
- + }
- elsif ($do eq 'prefs') {
- cgi_prefs($q, $session);
- }
- diff -urx .svn ikiwiki/IkiWiki/Rcs/svn.pm ikiwiki-new/IkiWiki/Rcs/svn.pm
- --- ikiwiki/IkiWiki/Rcs/svn.pm 2006-12-28 17:50:46.000000000 -0800
- +++ ikiwiki-new/IkiWiki/Rcs/svn.pm 2007-01-11 18:14:30.000000000 -0800
+ my $q=shift;
+
+ @@ -656,6 +756,9 @@
+ elsif (defined $session->param("postsignin")) {
+ cgi_postsignin($q, $session);
+ }
+ + elsif ($do eq 'move') {
+ + cgi_movepage($q, $session);
+ + }
+ elsif ($do eq 'prefs') {
+ cgi_prefs($q, $session);
+ }
+ diff -urNX ignorepats ikiwiki/IkiWiki/Rcs/svn.pm ikidev/IkiWiki/Rcs/svn.pm
+ --- ikiwiki/IkiWiki/Rcs/svn.pm 2007-01-27 16:04:48.000000000 -0800
+ +++ ikidev/IkiWiki/Rcs/svn.pm 2007-02-22 01:51:29.923626000 -0800
@@ -60,6 +60,34 @@
- }
+ }
} #}}}
-
+
+sub rcs_move ($$$$;$$) {
- + my $file=shift;
- + my $newname=shift;
- + my $message=shift;
- + my $rcstoken=shift;
- + my $user=shift;
- + my $ipaddr=shift;
- + if (defined $user) {
- + $message="web commit by $user".(length $message ? ": $message" : "");
- + }
- + elsif (defined $ipaddr) {
- + $message="web commit from $ipaddr".(length $message ? ": $message" : "");
- + }
+ + my $file=shift;
+ + my $newname=shift;
+ + my $message=shift;
+ + my $rcstoken=shift;
+ + my $user=shift;
+ + my $ipaddr=shift;
+ + if (defined $user) {
+ + $message="web commit by $user".(length $message ? ": $message" : "");
+ + }
+ + elsif (defined $ipaddr) {
+ + $message="web commit from $ipaddr".(length $message ? ": $message" : "");
+ + }
+
- + chdir($config{srcdir}); # svn merge wants to be here
+ + chdir($config{srcdir}); # svn merge wants to be here
+
- + if (system("svn", "move", "--quiet",
- + "$file", "$newname") != 0) {
- + return 1;
- + }
- + if (system("svn", "commit", "--quiet",
- + "--encoding", "UTF-8", "-m",
- + possibly_foolish_untaint($message)) != 0) {
- + return 1;
- + }
- + return undef # success
+ + if (system("svn", "move", "--quiet",
+ + "$file", "$newname") != 0) {
+ + return 1;
+ + }
+ + if (system("svn", "commit", "--quiet",
+ + "--encoding", "UTF-8", "-m",
+ + possibly_foolish_untaint($message)) != 0) {
+ + return 1;
+ + }
+ + return undef # success
+}
+
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.
+ # Tries to commit the page; returns undef on _success_ and
+ # a version of the page with the rcs's conflict markers on failure.
+ diff -urNX ignorepats ikiwiki/IkiWiki/Render.pm ikidev/IkiWiki/Render.pm
+ --- ikiwiki/IkiWiki/Render.pm 2007-02-14 17:00:05.000000000 -0800
+ +++ ikidev/IkiWiki/Render.pm 2007-02-22 18:30:00.451755000 -0800
+ @@ -80,6 +80,7 @@
+
+ if (length $config{cgiurl}) {
+ $template->param(editurl => cgiurl(do => "edit", page => $page));
+ + $template->param(moveurl => cgiurl(do => "move", page => $page));
+ $template->param(prefsurl => cgiurl(do => "prefs"));
+ if ($config{rcs}) {
+ $template->param(recentchangesurl => cgiurl(do => "recentchanges"));
+ diff -urNX ignorepats ikiwiki/templates/movepage.tmpl ikidev/templates/movepage.tmpl
+ --- ikiwiki/templates/movepage.tmpl 1969-12-31 16:00:00.000000000 -0800
+ +++ ikidev/templates/movepage.tmpl 2007-02-22 18:40:39.751763000 -0800
+ @@ -0,0 +1,44 @@
+ +
+ +
+ +
+ +
+ +
+ +<TMPL_VAR FORM-TITLE>
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +Failed to save your changes.
+ +
+ +
+ +Your changes were not able to be saved to disk. The system gave the error:
+ +
+ +
+ +
+ +Your changes are preserved below, and you can try again to save them.
+ +
+ +
+ +
+ +