]> sipb.mit.edu Git - ikiwiki.git/blobdiff - doc/todo/feed_enhancements_for_inline_pages.mdwn
review remaining patches
[ikiwiki.git] / doc / todo / feed_enhancements_for_inline_pages.mdwn
index 290326d9a5f48217eeba6fa76434102b4cd793ff..ee28ffa6e72366646d232028e65ce6f7533afc90 100644 (file)
@@ -17,6 +17,8 @@ A few patches to clean up and improve feed management for inline pages.
   >> \#blogform -> .blogform change. I have no idea on where to document
   >> this change though.
 
+  >>> Picked. NEWSed. --[[Joey]]
+
 * the second patch tries to define the default description for a feed based not only on the wiki name, but also on the current page name. The actual way this is built might not be the optimal one, so I'm open to suggestions
 
   > I don't really like using "wikiname/page" as the name of the feed. It's
@@ -32,6 +34,10 @@ A few patches to clean up and improve feed management for inline pages.
   >> description, then we use the page title combined with the wiki name. I introduce a new
   >> configuration key to customize the actual automatic description.
 
+  >>> The feed title part of this seems unnecessary. As far as I can see,
+  >>> ikiwiki already uses the page title as the feed title; TITLE in the
+  >>> rsspage.tmpl is handled the same as TITLE in page.tmpl. --[[Joey]]
+
 * the (former) third patch passes the feed titles to the templates, changing the default templates to use these as title attributes for the links. a rel="alternate" attribute is also included
 
   > Seems reasonable. Cherry-picked. Note that the title attribute
@@ -44,6 +50,19 @@ A few patches to clean up and improve feed management for inline pages.
   actually a forgotten piece from my previous patch (now upstream) to base the feed name on the
   included rather than the including page, and it's only relevant for nested inline pages.
 
+  > I have a vague memory of considering doing this before, and not,
+  > because there is actually no guarantee that the inlined page (that
+  > itself contains an inline) will generate an url. It could be excluded;
+  > it could be an internal page; it could use a conditional to omit the
+  > inline when not inlined.
+  > 
+  > Also, I think that `destpage` gets set wrong. And I think that
+  > `get_inline_content` is called with the source page, rather than the
+  > destpage, and so could generate urls that don't work on the destpage.
+  >
+  > All in all, this is an edge case, and currently seems to work ok, so
+  > why change it? --[[Joey]] 
+
 * the (former) fourth patch introduces a feedlinks parameter to the inline directive, to allow for the specifications of the locations where the feed links should appear. Currently, two options are allowed (head and body), plus both and none with obvious significance
 
   > Hmm. This doesn't affect the feed links in the blogform.tmpl. Anyway,
@@ -66,6 +85,27 @@ A few patches to clean up and improve feed management for inline pages.
   not squashing it with the second patch to allow you to scrap this but sitll get the other, in case
   you're not too happy about having a plethora of parameters
 
+  > This seems clearly a good idea, since there is already a "description"
+  > parameter. But, by analogy with that parameter, it should just be
+  > called "title". --[[Joey]] 
+
 * a fifth patch introduces an `id` parameter to allow setting the HTML id attribute in the
   blogpost/feedlinks template. Since we replace their id with a class (first patch), this brings
   back the possibility for direct CSS customization and JavaScript manipulation based on id.
+
+  > That sort of makes sense, but it somehow seems wrong that "id" should
+  > apply to only cruft at the top of the inline, and not the entire div
+  > generated for it. --[[Joey]] 
+
+* 30a4de2aa3ab29dd9397c2edd91676e80bc06feb "urlto: prevent // when {url} ends with /"
+
+  > The `url` in the setup file should not end in a slash. Probably more
+  > things get ugly doubled slashes if someone does that. --[[Joey]] 
+
+* 57a9b5c4affda9e855f09a64747e5225d6254079 "inline: use urlto instead of manually building the RSS url"
+
+  > Well, that seems ok. 3 parameter urlto should give us an absolute url.
+  > 
+  > But we have to be careful and verify that it will always produce
+  > exactly the same url as before. Changing the feed url unnecessarily
+  > can probably flood aggregators or something... --[[Joey]]