]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/todo/Moving_Pages.mdwn
Merge commit 'origin/master'
[git.ikiwiki.info.git] / doc / todo / Moving_Pages.mdwn
index e1447bfb7962d541119355683bf086c288cc0863..387e4fb82be1b7388cedaf630ee3047c87d6be2c 100644 (file)
@@ -10,6 +10,8 @@ Thanks again to [Joey](http://kitenet.net/~joey) for putting ikiwiki together.
 
 *[Kyle](http://kitenet.net/~kyle/)=*
 
 
 *[Kyle](http://kitenet.net/~kyle/)=*
 
+> Took a bit too long, but [[done]] now. --[[Joey]]
+
 ----
 
 The MediaWiki moving/renaming mechanism is pretty nice.  It's easy to get a list of pages that point to the current page.  When renaming a page it sticks a forwarding page in the original place.  The larger the size of the wiki the more important organization tools become.
 ----
 
 The MediaWiki moving/renaming mechanism is pretty nice.  It's easy to get a list of pages that point to the current page.  When renaming a page it sticks a forwarding page in the original place.  The larger the size of the wiki the more important organization tools become.
@@ -37,285 +39,6 @@ Brad
 
 -----
 
 
 -----
 
-[[!tag patch]]
-
-This is my second cut at a feature like that requested here.
-It can also be found [here](http://ikidev.betacantrips.com/patches/move.patch).
-
-A few shortcomings exist: 
-
-* No precautions whatsoever are made to protect against race conditions or failures
-  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. 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.
-* 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
-> implement a move operation -- falling back to an add and delete.
-> --[[Joey]]
-
-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?
-
-> 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 -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 page newdir newname comments);
-    +   my @buttons=("Rename Page", "Cancel");
-    +
-    +   my $form = CGI::FormBuilder->new(
-    +           fields => \@fields,
-    +                header => 1,
-    +                charset => "utf-8",
-    +                method => 'POST',
-    +           action => $config{cgiurl},
-    +                template => (-e "$config{templatedir}/movepage.tmpl" ?
-    +                        {template_params("movepage.tmpl")} : ""),
-    +   );
-    +   run_hooks(formbuilder_setup => sub {
-    +           shift->(form => $form, cgi => $q, session => $session);
-    +   });
-    +
-    +   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);
-    +
-    +   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 => "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) {
-    +           $form->field(name => "rcsinfo", value => rcs_prepedit($file),
-    +                        force => 1);
-    +   }
-    +
-    +   if ($form->submitted eq "Cancel") {
-    +           redirect($q, "$config{url}/".htmlpage($page));
-    +           return;
-    +   }
-    +
-    +   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;
-
-    @@ -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" : "");
-    +   }
-    +
-    +   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
-    +}
-    +
-     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.
-    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 @@
-    +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
-    + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
-    +<html>
-    +<head>
-    +<base href="<TMPL_VAR BASEURL>" />
-    +<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
-    +<title><TMPL_VAR FORM-TITLE></title>
-    +<link rel="stylesheet" href="<TMPL_VAR BASEURL>style.css" type="text/css" />
-    +<link rel="stylesheet" href="<TMPL_VAR BASEURL>local.css" type="text/css" />
-    +<TMPL_IF NAME="FAVICON">
-    +<link rel="icon" href="<TMPL_VAR BASEURL><TMPL_VAR FAVICON>" type="image/x-icon" />
-    +</TMPL_IF>
-    +</head>
-    +<body>
-    +<TMPL_IF NAME="FAILED_SAVE">
-    +<p>
-    +<b>Failed to save your changes.</b>
-    +</p>
-    +<p>
-    +Your changes were not able to be saved to disk. The system gave the error:
-    +<blockquote>
-    +<TMPL_VAR ERROR_MESSAGE>
-    +</blockquote>
-    +Your changes are preserved below, and you can try again to save them.
-    +</p>
-    +</TMPL_IF>
-    +<TMPL_VAR FORM-START>
-    +<div class="header">
-    +<span><TMPL_VAR INDEXLINK>/ <TMPL_VAR FORM-TITLE></span>
-    +</div>
-    +<TMPL_VAR FIELD-DO>
-    +<TMPL_VAR FIELD-FROM>
-    +<TMPL_VAR FIELD-RCSINFO>
-    +<TMPL_VAR FIELD-PAGE>
-    +New location: <TMPL_VAR FIELD-NEWDIR>/ <TMPL_VAR FIELD-NEWNAME>
-    +<br />
-    +<TMPL_IF NAME="CAN_COMMIT">
-    +Optional comment about this change:<br />
-    +<TMPL_VAR FIELD-COMMENTS><br />
-    +</TMPL_IF>
-    +<input id="_submit" name="_submit" type="submit" value="Rename Page" /><input id="_submit_2" name="_submit" type="submit" value="Cancel" />
-    +<TMPL_VAR FORM-END>
-    +</body>
-    +</html>
-    diff -urNX ignorepats ikiwiki/templates/page.tmpl ikidev/templates/page.tmpl
-    --- ikiwiki/templates/page.tmpl     2006-12-28 12:27:01.000000000 -0800
-    +++ ikidev/templates/page.tmpl      2007-02-22 01:52:33.078464000 -0800
-    @@ -32,6 +32,9 @@
-     <TMPL_IF NAME="EDITURL">
-     <li><a href="<TMPL_VAR EDITURL>">Edit</a></li>
-     </TMPL_IF>
-    +<TMPL_IF NAME="MOVEURL">
-    +<li><a href="<TMPL_VAR MOVEURL>">Move</a></li>
-    +</TMPL_IF>
-     <TMPL_IF NAME="RECENTCHANGESURL">
-     <li><a href="<TMPL_VAR RECENTCHANGESURL>">RecentChanges</a></li>
-     </TMPL_IF>
-
-----
-
 I'm going to try to run through a full analysis and design for moving and
 deleting pages here. I want to make sure all cases are covered. --[[Joey]]
 
 I'm going to try to run through a full analysis and design for moving and
 deleting pages here. I want to make sure all cases are covered. --[[Joey]]
 
@@ -361,14 +84,25 @@ It won't be possible to put it on the action bar or somewhere else. (It
 would be possible to code up a different rename button that doesn't do
 that, and use it elsewhere.)
 
 would be possible to code up a different rename button that doesn't do
 that, and use it elsewhere.)
 
+Hmm, unless it saves the edit state and reloads it later, while using a separate
+form. Which seems to solve other problems, so I think is the way to go.
+
 ## SubPages
 
 When renaming `foo`, it probably makes sense to also rename
 `foo/Discussion`. Should other SubPages in `foo/` also be renamed? I think
 it's probably simplest to rename all of its SubPages too.
 
 ## SubPages
 
 When renaming `foo`, it probably makes sense to also rename
 `foo/Discussion`. Should other SubPages in `foo/` also be renamed? I think
 it's probably simplest to rename all of its SubPages too.
 
+(For values of "simplest" that don't include the pain of dealing with all
+the changed links on subpages.. as well as issues like pagespecs that
+continue to match the old subpages, and cannot reasonably be auto-converted
+to use the new, etc, etc... So still undecided about this.)
+
 When deleting `foo`, I don't think SubPages should be deleted. The
 When deleting `foo`, I don't think SubPages should be deleted. The
-potential for mistakes and abuse is too large.
+potential for mistakes and abuse is too large. Deleting Discussion page
+might be a useful exception.
+
+TODO: Currently, subpages are not addressed.
 
 ## link fixups
 
 
 ## link fixups
 
@@ -376,6 +110,18 @@ When renaming a page, it's desirable to keep links that point to it
 working. Rather than use redirection pages, I think that all pages that
 link to it should be modified to fix their links.
 
 working. Rather than use redirection pages, I think that all pages that
 link to it should be modified to fix their links.
 
+The rename plugin can add a "rename" hook, which other plugins can use to
+update links &etc. The hook would be passed page content, the old and new
+link names, and would modify the content and return it. At least the link
+plugin should have such a hook.
+
+After calling the "rename" hook, and rendering the wiki, the rename plugin
+can check to see what links remain pointing to the old page. There could
+still be some, for example, CamelCase links probably won't be changed; img
+plugins and others contain logical links to the file, etc. The user can be
+presented with a list of all the pages that still have links to the old
+page, and can manually deal with them.
+
 In some cases, a redirection page will be wanted, to keep long-lived urls
 working. Since the meta plugin supports creating such pages, and since they
 won't always be needed, I think it will be simplest to just leave it up to
 In some cases, a redirection page will be wanted, to keep long-lived urls
 working. Since the meta plugin supports creating such pages, and since they
 won't always be needed, I think it will be simplest to just leave it up to
@@ -387,60 +133,90 @@ The source page must be editable by the user to be deleted/renamed.
 When renaming, the dest page must not already exist, and must be creatable
 by the user, too.
 
 When renaming, the dest page must not already exist, and must be creatable
 by the user, too.
 
-When deleting/renaming attachments, the `allowed_attachments` PageSpec
+lWhen deleting/renaming attachments, the `allowed_attachments` PageSpec
 is checked too.
 
 ## RCS
 
 is checked too.
 
 ## RCS
 
-Two new optional functions are added to the RCS interface:
-
-* `rcs_delete(file, message, rcstoken, user, ipaddr)`
-* `rcs_rename(old, new, message, rcstoken, user, ipaddr)`
+Three new functions are added to the RCS interface:
 
 
-The page move/rename code will check if these are not available, and error
-out.
+* `rcs_remove(file)`
+* `rcs_rename(old, new)`
+* `rcs_commit_staged(message, user, ip)`
 
 
-Similar to `rcs_commit` both of these take a rcstoken, which is generated
-by an earlier `rcs_prepedit`.
+See [[rcs_updates_needed_for_rename_and_remove]].
 
 ## conflicts
 
 
 ## conflicts
 
-Cases that have to be dealt with:
+Cases to consider:
 
 * Alice clicks "delete" button for a page; Bob makes a modification;
   Alice confirms deletion. Ideally in this case, Alice should get an error
   message that there's a conflict.
 
 * Alice clicks "delete" button for a page; Bob makes a modification;
   Alice confirms deletion. Ideally in this case, Alice should get an error
   message that there's a conflict.
+  Update: In my current code, alice's deletion will fail if the file was
+  moved or deleted in the meantime; if the file was modified since alice
+  clicked on the delete button, the modifications will be deleted too. I
+  think this is acceptable.
 * Alice opens edit UI for a page; Bob makes a modification; Alice
   clicks delete button and confirms deletion. Again here, Alice should get
   a conflict error. Note that this means that the rcstoken should be
   recorded when the edit UI is first opened, not when the delete button is
   hit.
 * Alice opens edit UI for a page; Bob makes a modification; Alice
   clicks delete button and confirms deletion. Again here, Alice should get
   a conflict error. Note that this means that the rcstoken should be
   recorded when the edit UI is first opened, not when the delete button is
   hit.
+  Update: Again here, there's no conflict, but the delete succeeds. Again,
+  basically acceptible.
 * Alice and Bob both try to delete a page at the same time. It's fine for
   the second one to get a message that it no longer exists. Or just to
   silently fail to delete the deleted page..
 * Alice and Bob both try to delete a page at the same time. It's fine for
   the second one to get a message that it no longer exists. Or just to
   silently fail to delete the deleted page..
+  Update: It will display an error to the second one that the page doesn't
+  exist.
 * Alice deletes a page; Bob had edit window open for it, and saves
   it afterwards. I think that Bob should win in this case; Alice can always
   notice the page has been added back, and delete it again.
 * Alice deletes a page; Bob had edit window open for it, and saves
   it afterwards. I think that Bob should win in this case; Alice can always
   notice the page has been added back, and delete it again.
+  Update: Bob wins.
 * Alice clicks "rename" button for a page; Bob makes a modification;
   Alice confirms rename. This case seems easy, it should just rename the
   modified page.
 * Alice clicks "rename" button for a page; Bob makes a modification;
   Alice confirms rename. This case seems easy, it should just rename the
   modified page.
+  Update: it does
 * Alice opens edit UI for a page; Bob makes a modification; Alice
   clicks rename button and confirms rename. Seems same as previous case.
 * Alice opens edit UI for a page; Bob makes a modification; Alice
   clicks rename button and confirms rename. Seems same as previous case.
+  Update: check
 * Alice and Bob both try to rename a page at the same time (to probably
   different names). Or one tries to delete, and the other to rename. 
   I think it's acceptible for the second one to get an error message that
   the page no longer exists.
 * Alice and Bob both try to rename a page at the same time (to probably
   different names). Or one tries to delete, and the other to rename. 
   I think it's acceptible for the second one to get an error message that
   the page no longer exists.
+  Update: check, that happens
 * Alice renames a page; Bob had edit window open for it, and saves
   it afterwards, under old name. I think it's acceptible for Bob to succeed
   in saving it under the old name in this case, though not ideal.
 * Alice renames a page; Bob had edit window open for it, and saves
   it afterwards, under old name. I think it's acceptible for Bob to succeed
   in saving it under the old name in this case, though not ideal.
-* Alice renames (or deletes) a page. In the meantime, Bob is uploading an
-  attachment to it, and finishes after the rename finishes. Is it
-  acceptible for the attachment to be saved under the old name?
+  Update: Behavior is the same as if Alice renamed the page and Bob created
+  a new page with the old name. Seems acceptable, though could be mildly
+  confusing to Bob (or Alice).
 * Alice starts creating a new page. In the meantime, Bob renames a
   different page to that name. Alice should get an error message when
   committing; and it should have conflict markers. Ie, this should work the
   same as if Bob had edited the new page at the same time as Alice did.
 * Alice starts creating a new page. In the meantime, Bob renames a
   different page to that name. Alice should get an error message when
   committing; and it should have conflict markers. Ie, this should work the
   same as if Bob had edited the new page at the same time as Alice did.
+  Update: That should happen. Haven't tested this case yet to make sure.
 * Bob starts renaming a page. In the meantime, Alice creates a new page
   with the name he's renaming it to. Here Bob should get a error message
   that he can't rename the page to an existing name. (A conflict resolution
   edit would also be ok.)
 * Bob starts renaming a page. In the meantime, Alice creates a new page
   with the name he's renaming it to. Here Bob should get a error message
   that he can't rename the page to an existing name. (A conflict resolution
   edit would also be ok.)
+  Update: Bob gets an error message.
+* Alice renames (or deletes) a page. In the meantime, Bob is uploading an
+  attachment to it, and finishes after the rename finishes. Is it
+  acceptible for the attachment to be saved under the old name?
+  Update: Meh. It's certianly not ideal; if Bob tries to save the page he
+  uploaded the attachment to, he'll get a message about it having been
+  deleted/renamed, and he can try to figure out what to do... :-/
+* I don't know if this is a conflict, but it is an important case to consider;
+  you need to make sure that there are no security holes.  You dont want
+  someone to be able to rename something to <code>/etc/passwd</code>.
+  I think it would be enough that you cannot rename to a location outside
+  of srcdir, you cannot rename to a location that you wouldn't be able
+  to edit because it is locked, and you cannot rename to an existing page.
+
+  > Well, there are a few more cases (like not renaming to a pruned
+  > filename, and not renaming _from_ a file that is not a known source
+  > file or is locked), but yes, that's essentially it.
+  > 
+  > PS, the first thing I do to any
+  > web form is type /etc/passwd and ../../../../etc/passwd into it. ;-) --[[Joey]]