doc/todo/feed_enhancements_for_inline_pages.mdwn

   1 [[!template id=gitbranch branch=GiuseppeBilotta/inlinestuff author="Giuseppe Bilotta"]]
   2
   3 A few patches to clean up and improve feed management for inline pages.
   4
   5 (I moved the picked/scratched stuff at the bottom.)
   6
   7 * the (now first) patch tries to define the default description for a feed based not only on the wiki name,
   8   but also on the current page name. The actual way this is built might not be the optimal one,
   9   so I'm open to suggestions
  10
  11   > I don't really like using "wikiname/page" as the name of the feed. It's
  12   > a bit too mechanical. I'd be ok with using just the page name,
  13   > with a fallback to wikiname for the toplevel index. Or maybe
  14   > something like "$wikiname's $page".
  15   >
  16   > Also, shouldn't `pagetitle` be run on the page name? (Haven't checked.)
  17   > --[[Joey]]
  18
  19   >> The rewritten patch now sets the feed title using the page title, and the feed description
  20   >> using the page _description_, both obtained from meta if possible. If there is no page
  21   >> description, then we use the page title combined with the wiki name. I introduce a new
  22   >> configuration key to customize the actual automatic description.
  23
  24   >>> The feed title part of this seems unnecessary. As far as I can see,
  25   >>> ikiwiki already uses the page title as the feed title; TITLE in the
  26   >>> rsspage.tmpl is handled the same as TITLE in page.tmpl. --[[Joey]]
  27
  28   >>>> I'm afraid this is not the case in the ikiwiki I have. It might be the effect of some kind of interaction of
  29   >>>> this with the next patch, but apparently I need both to ensure that the proper title is being used.
  30
  31   >>>> Some further analysis: before my patch, the feed title would be set to
  32   >>>> `pagetitle($page)`, or to the wiki name if the pagetitle was index. As
  33   >>>> it turns out, in my setup (see below for details) this happens quite
  34   >>>> often on my `dirN.mdwn` index pages, where I would like to have `dirN`
  35   >>>> as title instead. Plus, unless I'm mistaken, `pagetitle()` doesn't
  36   >>>> actually use `meta` information, which my patch does. So I still think
  37   >>>> the title part of the patch is worth it. As a bonus, it also allows title
  38   >>>> customization by the `title=` parameter as offered in another patch.
  39
  40 * the (now second) patch passes uses the included rather than the including page for the URL. This is
  41   actually a forgotten piece from my previous patch (now upstream) to base the feed name on the
  42   included rather than the including page, and it's only relevant for nested inline pages.
  43
  44   > I have a vague memory of considering doing this before, and not,
  45   > because there is actually no guarantee that the inlined page (that
  46   > itself contains an inline) will generate an url. It could be excluded;
  47   > it could be an internal page; it could use a conditional to omit the
  48   > inline when not inlined.
  49
  50   >> I would say that in this cases my patch wouldn't change anything because
  51   >> either the code would still act as before or it wouldn't be triggered at
  52   >> all. --GB
  53
  54   > Also, I think that `destpage` gets set wrong. And I think that
  55   > `get_inline_content` is called with the source page, rather than the
  56   > destpage, and so could generate urls that don't work on the destpage.
  57
  58   >> `destpage` getting set wrong is probably a bug that should be
  59   >> fixed, but I must say I haven't come across it (yet).
  60   >> `get_inline_content` is called with both the source and dest page,
  61   >> and in my experience the urls have always been generated correctly.
  62
  63   > All in all, this is an edge case, and currently seems to work ok, so
  64   > why change it? --[[Joey]]
  65
  66   >> Because it does not work ok for me. I have a number of directories `dir1/`, `dir2/`, `dir3/`
  67   >> each with a corresponding `dir1.mdwn`, `dir2.mdwn`, `dir3.mdwn` etc that is basically just
  68   >> an inline instruction. Then my index.mdwn inlines `dir[123]`. Without these two patches, the
  69   >> `dir[123]` feeds get the wrong title.
  70
  71 * the (new) fourth patch introduces a `feedtitle` parameter to override the feed title. I opted for
  72   not squashing it with the second patch to allow you to scrap this but still get the other, in case
  73   you're not too happy about having a plethora of parameters
  74
  75   > This seems clearly a good idea, since there is already a "description"
  76   > parameter. But, by analogy with that parameter, it should just be
  77   > called "title". --[[Joey]]
  78
  79   >> I'll rework the patch to that effect.
  80
  81 * a fifth patch introduces an `id` parameter to allow setting the HTML id attribute in the
  82   blogpost/feedlinks template. Since we replace their id with a class (first patch), this brings
  83   back the possibility for direct CSS customization and JavaScript manipulation based on id.
  84
  85   > That sort of makes sense, but it somehow seems wrong that "id" should
  86   > apply to only cruft at the top of the inline, and not the entire div
  87   > generated for it. --[[Joey]]
  88
  89   >> Good point. I'll look into a way to move the id to the `inlinepage` div, although I guess
  90   >> that falling back to `id`ing the `feedlink` div in the feedonly case would be ok.
  91
  92   >> After looking into it, I hit again the same naive error I did while
  93   >> working on inline the first time: there is no "outer" div that
  94   >> encloses all of the generated content: each inlined page has its
  95   >> "inlinepage"-classed div, and the lot of them is prefixed by either
  96   >> the feedlinks or postform template output. So the only way to "id"
  97   >> a whole block of inlines is by adding a wrapping div that encloses
  98   >> the whole product of the inline directive. I can do that if you
  99   >> believe it's worth it.
 100
 101 * 30a4de2aa3ab29dd9397c2edd91676e80bc06feb "urlto: prevent // when {url} ends with /"
 102
 103   > The `url` in the setup file should not end in a slash. Probably more
 104   > things get ugly doubled slashes if someone does that. --[[Joey]]
 105
 106   >> I was not aware of this. Did I miss it or is it just not documented?
 107   >> Also, grepping through the current official code (core and plugins)
 108   >> there is only one other place that looks like it could be affected
 109   >> by the `url` config ending in slash, and it's the `$local_url`
 110   >> stuff in `IkiWiki.pm`, but that code does terminal double-slash
 111   >> sanitation itself. So it would seem that my proposed patch would
 112   >> lift the restriction about the terminal / (an otherwise unnecessary
 113   >> restriction) without affecting much, as long as `url` users rely on
 114   >> the core functions to build paths with it (as in the next patch).
 115
 116 * 57a9b5c4affda9e855f09a64747e5225d6254079 "inline: use urlto instead of manually building the RSS url"
 117
 118   > Well, that seems ok. 3 parameter urlto should give us an absolute url.
 119   >
 120   > But we have to be careful and verify that it will always produce
 121   > exactly the same url as before. Changing the feed url unnecessarily
 122   > can probably flood aggregators or something... --[[Joey]]
 123
 124   >> AFAICS, the feed url would only change in the case of /-terminating
 125   >> `$config{url}`, and even then only if the preceding urlto sanitation patch
 126   >> was included too.
 127
 128
 129 -----
 130
 131 * the first patch simply replaces the id attribute in the default template for feedlinks with a class attribute by the same name. This is necessary in pages with multiple inlines to guarantee correctness
 132
 133   > Ok, but blogform.tmpl has the same problem. And either change can need
 134   > CSS changes. (blogform in particular is used in style.css as an id.)
 135   > So this needs more documentation and associated work. --[[Joey]]
 136
 137   >> I didn't include blogform in the change because the case of two
 138   >> blog post forms in the same page is probably extremely rare. But
 139   >> then again I remember doing having them in one of my ikiwiki
 140   >> draftings, so I rewrote the patch to include blogform. I had
 141   >> checked the distributed CSS for #feedlinks references, without
 142   >> finding any. The new patch does include CSS changes for the
 143   >> \#blogform -> .blogform change. I have no idea on where to document
 144   >> this change though.
 145
 146   >>> Picked. NEWSed. --[[Joey]]
 147
 148
 149 * 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
 150
 151   > Seems reasonable. Cherry-picked. Note that the title attribute
 152   > will be shown by browsers as a tooltip. So I made it say
 153   > "$name (RSS feed)"
 154
 155   >> Good, thanks.
 156
 157 * 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
 158
 159   > Hmm. This doesn't affect the feed links in the blogform.tmpl. Anyway,
 160   > this is not something I see a real benefit of making configurable above
 161   > the template editing level. I don't see any point whatsoever of
 162   > allowing to turn off the feed links in the `<head>` -- they are not
 163   > user-visible, and IIRC that is the recommended and most portable way
 164   > to encode the information for feed discovery agents (rather than
 165   > putting it in the body). And the sorry state of "modern" browsers,
 166   > such as chromium's support for RSS means that it still makes sense to
 167   > have user-visible feed buttons. If that changed, it would make sense to
 168   > modify ikiwiki to globally remove them. --[[Joey]]
 169
 170   >> I was actually quite surprised myself by the lack of automatic feed
 171   >> discovery in chromium (although I noticed there's a sort-of
 172   >> official plugin to do it). Overall, I believe your critique is
 173   >> well-founded, I'll scratch this patch.
 174