aboutsummaryrefslogtreecommitdiff
path: root/doc/bugs/trail_excess_dependencies.mdwn
diff options
context:
space:
mode:
Diffstat (limited to 'doc/bugs/trail_excess_dependencies.mdwn')
-rw-r--r--doc/bugs/trail_excess_dependencies.mdwn95
1 files changed, 95 insertions, 0 deletions
diff --git a/doc/bugs/trail_excess_dependencies.mdwn b/doc/bugs/trail_excess_dependencies.mdwn
new file mode 100644
index 000000000..f806a62eb
--- /dev/null
+++ b/doc/bugs/trail_excess_dependencies.mdwn
@@ -0,0 +1,95 @@
+I've just modified the trail plugin to use only presence, and not
+content dependencies. Using content dependencies, particularly to the page
+that defines the trail, meant that every time that page changed, *every*
+page in the trail gets rebuilt. This leads to users setting up sites that
+have horrible performance, if the trail is defined in, for example, the top
+page of a blog.
+
+Unfortunatly, this change to presence dependencies has
+introduced a bug. Now when an existing trail is removed, the pages in the
+trail don't get rebuilt to remove the trail (both html display and state).
+
+> Actually, this particular case is usually OK. Suppose a trail `untrail`
+> contains `untrail/a` (as is the case in the regression
+> test I'm writing), and you build the wiki, then edit `untrail` to no
+> longer be a trail, and refresh. `untrail` has changed, so it is
+> rendered. Assuming that the template of either `untrail` or another
+> changed page happens to contain the `TRAILS` variable (which is not
+> guaranteed, but is highly likely), `I::P::t::prerender`
+> is invoked. It notices that `untrail/a` was previously a trail
+> member and is no longer, and rebuilds it with the diagnostic
+> "building untrail/a, its previous or next page has changed".
+>
+> Strictly speaking, I should change `I::P::t::build_affected`
+> so it calls `prerender`, so we're guaranteed to have done the
+> recalculation. Fixed in my branch. --[[smcv]]
+
+I think that to fix this bug, the plugin should use a hook to
+force rebuilding of all the pages that were in the trail, when
+the trail is removed (or changed).
+
+> The case of "the trail is changed" is still broken:
+> if the order of items changes, or the trail is removed,
+> then the logic above means it's OK, but if you
+> change the `\[[!meta title]]` of the trail, or anything else
+> used in the prev/up/next bar, the items won't show that
+> change. Fixed in my branch. --[[smcv]]
+
+There's a difficulty in doing that: The needsbuild hook runs before the scan
+hook, so before it has a chance to see if the trail directive is still there.
+It'd need some changes to ikiwiki's hooks.
+
+> That's what `build_affected` is for, and trail already used it. --s
+
+(An improvement in this area would probably simplify other plugins, which
+currently abuse the needsbuild hook to unset state, to handle the case
+where the directive that resulted in that state is removed.)
+
+I apologise for introducing a known bug, but the dependency mess was too
+bad to leave as-is. And I have very little time (and regrettably, even less
+power) to deal with it right now. :( --[[Joey]]
+
+[[!template id=gitbranch branch=smcv/ready/trail author="[[Simon_McVittie|smcv]]"]]
+[[!tag patch]]
+
+> I believe my `ready/trail` branch fixes this. There are regression tests.
+>
+> Here is an analysis of how the trail pages interdepend.
+>
+> * If *trail* contains a page *member* which does exist, *member* depends
+> on *trail*. This is so that if the trail directive is deleted from
+> *trail*, or if *trail*'s "friendly" title or trail settings are changed,
+> the trail navigation bar in *member* will pick up that change. This is
+> now only a presence dependency, which isn't enough to make those happen
+> correctly. [Edited to add: actually, the title is the only thing that
+> can affect *member* without affecting the order of members.]
+>
+> * If *trail* contains consecutive pages *m1* and *m2* in that order,
+> *m1* and *m2* depend on each other. This is so that if one's
+> "friendly" title changes, the other is rebuilt. This is now only
+> a presence dependency, which isn't enough to make those happen
+> correctly. In my branch, I explicitly track the "friendly" title
+> for every page that's edited and is involved in a trail somehow.
+>
+> * If *trail* has *member* in its `pagenames` but there is no page called
+> *member*, then *trail* must be rebuilt if *member* is created. This
+> was always a presence dependency, and is fine.
+>
+> In addition, the `trail` plugin remembers the maps
+> { trail => next item in that trail } and { trail => previous item in
+> that trail } for each page. If either changes, the page gets rebuilt
+> by `build_affected`, with almost the same logic as is used to update
+> pages that link to a changed page. My branch extends this to track the
+> "friendly title" of each page involved in a trail, either by being
+> the trail itself or a member (or both).
+>
+> I think it's true to say that the trail always depends on every member,
+> even if it doesn't display them. This might mean that we can use
+> "render the trail page" as an opportunity to work out whether any of
+> its members are also going to need re-rendering?
+> [Edited to add: actually, I didn't need this to be true, but I made the
+> regression test check it anyway.]
+>
+> --[[smcv]]
+
+>>> Thanks **very** much! [[done]] --[[Joey]]