]> sipb.mit.edu Git - ikiwiki.git/commitdiff
Merge commit 'origin/comments' into comments-joey
authorSimon McVittie <smcv@ http://smcv.pseudorandom.co.uk/>
Sun, 21 Dec 2008 10:17:45 +0000 (10:17 +0000)
committerSimon McVittie <smcv@ http://smcv.pseudorandom.co.uk/>
Sun, 21 Dec 2008 10:17:45 +0000 (10:17 +0000)
IkiWiki/Plugin/meta.pm
debian/changelog
doc/bugs/inline_sort_order_and_meta_date_value.mdwn
doc/todo/comments.mdwn

index ea60be5071f084321346db808e9f99b9b957bb5c..8c214139f8b85f3630055a7b954213ec9645ebbf 100644 (file)
@@ -121,6 +121,13 @@ sub preprocess (@) {
                $pagestate{$page}{meta}{authorurl}=$value if safeurl($value);
                # fallthrough
        }
+       elsif ($key eq 'date') {
+               eval q{use Date::Parse};
+               if (! $@) {
+                       my $time = str2time($value);
+                       $IkiWiki::pagectime{$page}=$time if defined $time;
+               }
+       }
 
        if (! defined wantarray) {
                # avoid collecting duplicate data during scan pass
@@ -128,14 +135,7 @@ sub preprocess (@) {
        }
 
        # Metadata collection that happens only during preprocessing pass.
-       if ($key eq 'date') {
-               eval q{use Date::Parse};
-               if (! $@) {
-                       my $time = str2time($value);
-                       $IkiWiki::pagectime{$page}=$time if defined $time;
-               }
-       }
-       elsif ($key eq 'permalink') {
+       if ($key eq 'permalink') {
                if (safeurl($value)) {
                        $pagestate{$page}{meta}{permalink}=$value;
                        push @{$metaheaders{$page}}, scrub('<link rel="bookmark" href="'.encode_entities($value).'" />', $destpage);
index d8685db09d26c3b202e6fc3d334709a9258ed7c4..056fabc7d4adf8a73ac56d44cd5e9cbd82ad8bad 100644 (file)
@@ -23,6 +23,8 @@ ikiwiki (2.71) UNRELEASED; urgency=low
   * aggregate: If a feed fails to be downloaded, try again immediatly
     next time aggregation is run, even if the usual time has not passed.
     Closes: #508622 (Michael Gold)
+  * meta: Process meta date during scan pass so that the date will always
+    affect sorting in inlines.
 
  -- Joey Hess <joeyh@debian.org>  Mon, 17 Nov 2008 14:02:10 -0500
 
index d851ef39742c3ff4334d94979cef1ea1c3d01376..d4ec8f345e5fc3b7348742ec50559f9c3a9473da 100644 (file)
@@ -99,3 +99,216 @@ first, explains everything you describe. If not, please send me a shell script
 test case I can run, as instructions like "Create pages that sort different by
 mtime and ctime" are not ones that I know how to follow (given that touch sets
 *both*). --[[Joey]] 
+
+> Sorry. I conflated Unix ctime and ikiwiki's creation time because I
+> didn't think the difference was important to this case. I'm a writer,
+> and I should have known better -- I appologize. Revised steps to
+> reproduce are below; feel free to delete this whole misunderstanding
+> to make the bug report more concise.
+>
+> 1. Create pages in the srcdir that should sort in one order by
+>    last-modification time and in a diffent order by original creation
+>    time.  For example:
+> 
+>         $ echo -e '\[[!meta date="2007-01-01"]]\nNew.' > test/new.mdwn
+>         $ echo -e '\[[!meta date="2006-01-01"]]\nOld.' > test/old.mdwn
+> 
+> 2. Create a page that includes the files.  For example:
+> 
+>
+>         $ echo '\[[!inline pages="test/*"]]' > sort-test.mdwn
+> 
+> 3. Run ikiwiki.  For example `ikiwiki --setup setup_file`
+> 
+> 4. Pages are output incorrectly in modification time order. For example,
+>    actual partial HTML of the sort-test/index.html from commands used
+>    above (whitespace-only lines removed; one whitespace-only line
+>    added):
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/old/">old</a>
+>         </span>
+>         <p>Old.</p>
+>         <span class="pagedate">
+>         Posted Sun 01 Jan 2006 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/new/">new</a>
+>         </span>
+>         <p>New.</p>
+>         <span class="pagedate">
+>         Posted Mon 01 Jan 2007 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+> 5. Run ikiwiki again with the same command line.  For example: `ikiwiki --setup setup_file`
+> 
+> 6. Pages are output correctly in creation time order. For example,
+>    actual partial HTML of the sort-test/index.html from commands used
+>    above (whitespace-only lines removed; one whitespace-only line
+>    added):
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/new/">new</a>
+>         </span>
+>         <p>New.</p>
+>         <span class="pagedate">
+>         Posted Mon 01 Jan 2007 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/old/">old</a>
+>         </span>
+>         <p>Old.</p>
+>         <span class="pagedate">
+>         Posted Sun 01 Jan 2006 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+> 7. Create a new page with the current Unix mtime and Unix ctime, but a
+>    !meta date before the most recent creation date of another page.
+>    For example:
+> 
+>         $ echo -e '\[[!meta date="2005-01-01"]]\nOlder.' > test/older.mdwn
+> 
+> 8. Run ikiwiki again with the same command line.  For example: `ikiwiki --setup setup_file`
+> 
+> 9. All previously sorted pages output correctly in order of their
+>    creation time, but the new page is output incorrectly at the top as
+>    if its modification time was its creation time.  For example,
+>    actual partial HTML of the sort-test/index.html from commands used
+>    above (whitespace-only lines removed; two whitespace-only
+>    lines added):
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/older/">older</a>
+>         </span>
+>         <p>Older.</p>
+>         <span class="pagedate">
+>         Posted Sat 01 Jan 2005 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/new/">new</a>
+>         </span>
+>         <p>New.</p>
+>         <span class="pagedate">
+>         Posted Mon 01 Jan 2007 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/old/">old</a>
+>         </span>
+>         <p>Old.</p>
+>         <span class="pagedate">
+>         Posted Sun 01 Jan 2006 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+> 10. Run ikiwiki again with the same command line.  For example: `ikiwiki --setup setup_file`
+> 
+> 11. All pages, including new page, are output correctly in creation time
+>     order. For example, actual partial HTML of the sort-test/index.html
+>     from commands used above (whitespace-only lines removed; two
+>     whitespace-only lines added): 
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/new/">new</a>
+>         </span>
+>         <p>New.</p>
+>         <span class="pagedate">
+>         Posted Mon 01 Jan 2007 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/old/">old</a>
+>         </span>
+>         <p>Old.</p>
+>         <span class="pagedate">
+>         Posted Sun 01 Jan 2006 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+>         <div class="inlinepage">
+>         <span class="header">
+>         <a href="./../test/older/">older</a>
+>         </span>
+>         <p>Older.</p>
+>         <span class="pagedate">
+>         Posted Sat 01 Jan 2005 12:00:00 AM EST
+>         </span>
+>         </div>
+> 
+> File status after all the above actions:
+> 
+>         $ stat test/*
+>           File: `test/new.mdwn'
+>           Size: 33              Blocks: 8          IO Block: 4096   regular file
+>         Device: ca20h/51744d    Inode: 684160      Links: 1
+>         Access: (0644/-rw-r--r--)  Uid: ( 1000/ harding)   Gid: ( 1000/ harding)
+>         Access: 2008-12-20 21:48:32.000000000 -0500
+>         Modify: 2008-12-20 21:27:03.000000000 -0500
+>         Change: 2008-12-20 21:27:03.000000000 -0500
+>           File: `test/older.mdwn'
+>           Size: 35              Blocks: 8          IO Block: 4096   regular file
+>         Device: ca20h/51744d    Inode: 684407      Links: 1
+>         Access: (0644/-rw-r--r--)  Uid: ( 1000/ harding)   Gid: ( 1000/ harding)
+>         Access: 2008-12-20 21:48:32.000000000 -0500
+>         Modify: 2008-12-20 21:42:10.000000000 -0500
+>         Change: 2008-12-20 21:42:10.000000000 -0500
+>           File: `test/old.mdwn'
+>           Size: 33              Blocks: 8          IO Block: 4096   regular file
+>         Device: ca20h/51744d    Inode: 684161      Links: 1
+>         Access: (0644/-rw-r--r--)  Uid: ( 1000/ harding)   Gid: ( 1000/ harding)
+>         Access: 2008-12-20 21:48:32.000000000 -0500
+>         Modify: 2008-12-20 21:27:09.000000000 -0500
+>         Change: 2008-12-20 21:27:09.000000000 -0500
+> 
+> My ikiwiki configuration file (being used to port a blog from pyblosxom
+> to ikiwiki):
+> 
+>         harding@mail:~$ sed 's/#.*//; /^[       ]*$/d' .ikiwiki/gnuisance.setup 
+>         use IkiWiki::Setup::Standard {
+>               wikiname => "HardingBlog",
+>               adminemail => 'dave@dtrt.org',
+>               srcdir => "/srv/backup/git/gnuisance.net",
+>               destdir => "/srv/test.dtrt.org",
+>               url => "http://srv.dtrt.org",
+>               wrappers => [
+>               ],
+>               atom => 1,
+>               syslog => 0,
+>               prefix_directives => 1,
+>               add_plugins => [qw{goodstuff tag}],
+>               disable_plugins => [qw{passwordauth}],
+>               tagbase => "tag",
+>         }
+>
+> --[David A. Harding](http://dtrt.org/), 2008-12-20
+
+Thank you for a textbook excellent reproduction recipe.
+
+What appears to be going on here is that meta directives are not processed
+until the leaf pages are rendered, and thus the ctime setting is not
+available at the time that they are inlined, and the newer unix ctime is
+used. On the second build, the meta data has already been recorded.
+
+This can probably be avoided by processing meta date at scan time.
+
+Verified, fix works. [[done]]
+--[[Joey]] 
index 12aef0bb3968f7f9eaf0a707f4ebbfdb226ae828..4edc3c3a12df88259716ff9f98818f86cc20de62 100644 (file)
@@ -1,22 +1,29 @@
-Known issues with the [[plugins/comments]] plugin:
+# Known issues with the [[plugins/comments]] plugin
 
-* There is some common code cargo-culted from other plugins (notably inline and editpage) which
-  should probably be shared
-
-  > Actually, there's less of this now than there used to be - a lot of simple
-  > things that were shared have become unshareable as they became more
-  > complex. --[[smcv]]
+## Unimplemented
 
 * Previews always say "unknown IP address"
 
-* Add `COMMENTOPENID`: the authenticated/verified user name, if and only if it was an OpenID
+* Instead of just a link to add a comment, it could have a form to enter
+  the title, similar to the form for adding a new blog post.
 
-  > Done in my comments git branch --[[smcv]]
+  > I'm not sure this is so useful? On Livejournal titles are allowed on
+  > comments, but very rarely used (and indeed usually not very useful);
+  > it's hard enough to get some people to title their blog posts :-)
+  > --[[smcv]]
 
-  > Not seeing it there, which branch? --[[Joey]]
+* If a spammer posts a comment, it is either impossible or hard to clean
+  up via the web. Would be nice to have some kind of link on the comment
+  that allows trusted users to remove it (using the remove plugin of
+  course).
 
-  >> Bah, git push --all is not the default... 'comments' branch now (I've also rebased it).
-  >> Sorry, I'm on mobile Internet at the moment... --[[smcv]]
+  > Won't the remove plugin refuse to remove internal pages? This would be
+  > a good feature to have, though. --[[smcv]]
+
+* Now that inline has some comments-specific functionality anyway, it would
+  be good to output `<link rel="comments">` in Atom and the equivalent in RSS.
+
+## Patches pending merge
 
 * The default template should have a (?) icon next to unauthenticated users (with the IP address
   as title) and an OpenID icon next to OpenIDs
@@ -29,49 +36,38 @@ Known issues with the [[plugins/comments]] plugin:
   >> directory (untested!) --[[smcv]]
 
   >>> The new code produces links like /wikiisons/openid.png, which
-  >>> fail if ikiwiki is not at the root of the web server. --[[Joey]] 
+  >>> fail if ikiwiki is not at the root of the web server. --[[Joey]]
+
+  >>>> Sorry, I should have spotted that (the assumption failed on my demo
+  >>>> site, but the push to that site was when I was on the way out, so I
+  >>>> didn't have time to investigate). As a note for other ikiwiki hackers,
+  >>>> I should have used
+  >>>> `<img src="<TMPL_VAR NAME=BASEURL>wikiicons/openid.png" />`. --[[smcv]]
 
   >>> I got to wondering if the icons are needed. On my comments branch
   >>> (not master), I've dropped the icons and info can be seen by hovering
   >>> over the author's name. Idea being that you probably don't care how
   >>> they authenticated unless something is weird, and in that case you
   >>> can hover to check. Does that make sense, should I merge it?
-  >>> --[[Joey]] 
-
-* Should the comments be visually set off more from the page above?
-  Rather than just a horizontal rule, I'm thinking put the comments
-  in a box like is used for inlined pages.
-
-  > I did put them in a box in the CSS... I agree the default template
-  > could do with visual improvement though. --[[smcv]]
+  >>> --[[Joey]]
 
-* Instead of just a link to add a comment, it could have a form to enter
-  the title, similar to the form for adding a new blog post.
-
-  > I'm not sure this is so useful? On Livejournal titles are allowed on
-  > comments, but very rarely used (and indeed usually not very useful);
-  > it's hard enough to get some people to title their blog posts :-)
-  > --[[smcv]]
-
-* If a spammer posts a comment, it is either impossible or hard to clean
-  up via the web. Would be nice to have some kind of link on the comment
-  that allows trusted users to remove it (using the remove plugin of
-  course).
+  >>>> Yeah, go ahead. I preferred my layout with the author before the
+  >>>> comment - perhaps that's Livejournal's influence :-) - but I can always
+  >>>> edit the templates for my own site. As long as the default is something
+  >>>> reasonable and both layouts are possible, I don't really mind.
+  >>>> Minimizing the number of "resource" files in the basewiki also seems
+  >>>> a good goal. --[[smcv]]
 
-  > Won't the remove plugin refuse to remove internal pages? This would be
-  > a good feature to have, though. --[[smcv]]
+## Won't fix
 
-* One can use inline to set up a feed of all comments posted to any page.
-  Using template=comment they are displayed right. Only problem
-  is there is no indication in that template of what page each comment in the
-  feed is a comment on. So, if a comment is inlined into a different page,
-  I think it should show a link back to the page commented on.
-  (BTW, the rss feed in this situation seems ok; there the link element
-  points back to the parent page.
+* There is some common code cargo-culted from other plugins (notably inline and editpage) which
+  should probably be shared
 
-  > done --[[Joey]] 
+  > Actually, there's less of this now than there used to be - a lot of simple
+  > things that were shared have become unshareable as they became more
+  > complex. --[[smcv]]
 
-* It would be useful to have a pagespec that always matches all comments on 
+* It would be useful to have a pagespec that always matches all comments on
   pages matching a glob. Something like `comment(blog/*)`.
   Perhaps postcomment could also be folded into this? Then the pagespec
   would match both existing comments, as well as new comments that are
@@ -89,17 +85,46 @@ Known issues with the [[plugins/comments]] plugin:
   > would also let X edit/delete comments on blog pages (including those
   > written by others) in arbitrary ways, which doesn't seem good. --[[smcv]]
 
-  > I had a look at implementing comment() and fell afoul of 
+  > I had a look at implementing comment() and fell afoul of
   > some optimisations that assume only internal() will be used to match
-  > internal pages. So probably this isn't worth doing. --[[Joey]] 
+  > internal pages. So probably this isn't worth doing. --[[Joey]]
+
+## Done
+
+* Add `COMMENTOPENID`: the authenticated/verified user name, if and only if it was an OpenID
+
+  > Done in my comments git branch --[[smcv]]
+
+  > Not seeing it there, which branch? --[[Joey]]
+
+  >> Bah, git push --all is not the default... 'comments' branch now (I've also rebased it).
+  >> Sorry, I'm on mobile Internet at the moment... --[[smcv]]
+
+  >>> merged by [[Joey]] in commit 0f03af38 --[[smcv]]
+
+* Should the comments be visually set off more from the page above?
+  Rather than just a horizontal rule, I'm thinking put the comments
+  in a box like is used for inlined pages.
+
+  > I did put them in a box in the CSS... I agree the default template
+  > could do with visual improvement though. --[[smcv]]
+
+  >> I'll consider this solved by [[Joey]]'s changes. --[[smcv]]
+
+* One can use inline to set up a feed of all comments posted to any page.
+  Using template=comment they are displayed right. Only problem
+  is there is no indication in that template of what page each comment in the
+  feed is a comment on. So, if a comment is inlined into a different page,
+  I think it should show a link back to the page commented on.
+  (BTW, the rss feed in this situation seems ok; there the link element
+  points back to the parent page.
+
+  > done --[[Joey]]
 
 * One of Joey's commit messages says "Not ideal, it would be nicer to jump to
   the actual comment posted, but no anchor is available". In fact there is
-  an anchor - the `\[[_comment]]` preprocessing wraps the comment in a <div>
+  an anchor - the `\[[_comment]]` preprocessing wraps the comment in a `<div>`
   with id="comment_123" or something. I'll fix this, unless Joey gets there
   first. --[[smcv]]
 
-  > done --[[Joey]] 
-
-* Now that inline has some comments-specific functionality anyway, it would
-  be good to output '<link rel="comments">' in Atom and the equivalent in RSS.
+  > done --[[Joey]]