7598ed21919f89c944ab6ab2b49ab046348e5aad
[ikiwiki.git] / doc / plugins / trail / discussion.mdwn
1 I believe the `trail3-integrated` and `trail3-prebuild` branches address
2 Joey's review comments from IRC:
3
4     06-12-2011 19:01:07 <joeyh>: ok, light review finished. so, if you want
5     to make a branch with inline trail=yes, and perhaps also adding a hook
6     so you don't need to inject, I think I can merge it right away
7
8 I haven't published instructions for using this version as a
9 standalone plugin, because it needs core and inline changes.
10
11 Commits up to 63bb8b42 make the trail plugin better-integrated,
12 including `\[[!inline trail=yes]]`. 63bb8b42 is the commit to
13 merge if you don't like the design of my hooks.
14
15 Commit 24168b99 adds a `build_affected` hook, run at about the
16 same time as `render_backlinks`, and uses it to render the
17 extra pages. This removes the need for `trail` to inject
18 anything. In principle, backlinks etc. could use this hook
19 too, if they weren't core.
20
21 Commit d0dea308 on the `trail3-prebuild` branch adds a
22 `prebuild` hook, which runs after everything has been scanned
23 but before anything is rendered. This removes the need
24 for `trail` to run its old `prerender` function in its
25 render hooks (preprocess, pagetemplate etc.) to collate
26 metadata before it renders anything. However, I'm not sure
27 that this is really the right thing to do, which is why it's
28 in its own branch: the `prebuild` hook is a lot like
29 `needsbuild` (but later), so it's called even if no trail
30 or trail member has actually been edited.
31
32 For it to be useful for `trail`, the `prebuild` hook has to run
33 after both pagespecs and sorting work. The other use case
34 I've seen for a similar hook was for Guiseppe Bilotta to
35 sort an inline-of-inlines by mtime of newest post, but that
36 can't be the same hook, because it has to run after pagespecs
37 work, but before sorting.
38
39 --[[smcv]]
40
41 > I've merged trail3-integrated, but not prebuild. I don't exactly dislike
42 > prebuild, but dunno that the hook prolieration is worth the minor cleanup
43 > it allows in trail. --[[Joey]]
44
45 >> Hmm, t/trail.t is failing several tests here. To reproduce, I build the
46 >> debian package from a clean state, or `rm -rf .t` between test runs. --[[Joey]]
47
48 <pre>
49 t/trail.t .................... 1/? 
50 #   Failed test at t/trail.t line 211.
51 #   Failed test at t/trail.t line 213.
52 #   Failed test at t/trail.t line 215.
53 #   Failed test at t/trail.t line 217.
54 #   Failed test at t/trail.t line 219.
55 #   Failed test at t/trail.t line 221.
56 #   Failed test at t/trail.t line 223.
57 #   Failed test at t/trail.t line 225.
58 #   Failed test at t/trail.t line 227.
59 #   Failed test at t/trail.t line 229.
60 #   Failed test at t/trail.t line 231.
61 </pre>
62
63 > Looking at the first of these, it expected "trail=sorting n=sorting/new p="
64 > but gets: "trail=sorting n=sorting/ancient p=sorting/new"
65 >
66 > Looking at the second failure, it expected "trail=sorting n=sorting/middle p=sorting/old$"
67 > but got: "trail=sorting n=sorting/old p=sorting/end"
68
69 > Perhaps a legitimate bug? --[[Joey]] 
70
71 >> I saw this while developing, but couldn't reproduce it, and assumed
72 >> I'd failed to update `blib` before `make test`, or some such.
73 >> In fact it's a race condition, I think.
74 >>
75 >> The change and failure here is that `sorting.mdwn` is modified
76 >> to sort its trail in reverse order of title. Previously, it
77 >> was sorted by order of directives in the page, and secondarily
78 >> by whatever sort order each directive specified (e.g.
79 >> new, old and ancient were sorted by increasing age).
80 >> `old` appearing between `new` and `ancient`, and `new` appearing
81 >> between `end` and `old`, indicates that this re-sorting has not
82 >> actually taken effect, and the old sort order is still used.
83 >>
84 >> I believe this is because the system time (as an integer) remained
85 >> the same for the entire test, and mtimes as used in ikiwiki
86 >> only have a 1-second resolution. We can either fix this with
87 >> utime or sleep; I chose utime, since sleeping for 1 second would
88 >> slow down the test significantly. Please merge or cherry-pick
89 >> `smcv/trail-test` (there's only one commit). --[[smcv]]
90
91 ----
92
93 [[!template id=gitbranch branch=smcv/ready/trail author=smcv]]
94
95 Some later changes to trail:
96
97 * Display the trail links at beginning/end of default `page.tmpl`
98   as suggested on IRC
99 * Improve CSS, particularly in blueview and goldtype themes
100   ([example](http://blueview.hosted.pseudorandom.co.uk/posts/second_post/))
101 * Fix a possible bug regarding state deletion
102
103 --[[smcv]]
104
105 > Applied --[[Joey]]