106 lines
4.3 KiB
Markdown
106 lines
4.3 KiB
Markdown
I believe the `trail3-integrated` and `trail3-prebuild` branches address
|
|
Joey's review comments from IRC:
|
|
|
|
06-12-2011 19:01:07 <joeyh>: ok, light review finished. so, if you want
|
|
to make a branch with inline trail=yes, and perhaps also adding a hook
|
|
so you don't need to inject, I think I can merge it right away
|
|
|
|
I haven't published instructions for using this version as a
|
|
standalone plugin, because it needs core and inline changes.
|
|
|
|
Commits up to 63bb8b42 make the trail plugin better-integrated,
|
|
including `\[[!inline trail=yes]]`. 63bb8b42 is the commit to
|
|
merge if you don't like the design of my hooks.
|
|
|
|
Commit 24168b99 adds a `build_affected` hook, run at about the
|
|
same time as `render_backlinks`, and uses it to render the
|
|
extra pages. This removes the need for `trail` to inject
|
|
anything. In principle, backlinks etc. could use this hook
|
|
too, if they weren't core.
|
|
|
|
Commit d0dea308 on the `trail3-prebuild` branch adds a
|
|
`prebuild` hook, which runs after everything has been scanned
|
|
but before anything is rendered. This removes the need
|
|
for `trail` to run its old `prerender` function in its
|
|
render hooks (preprocess, pagetemplate etc.) to collate
|
|
metadata before it renders anything. However, I'm not sure
|
|
that this is really the right thing to do, which is why it's
|
|
in its own branch: the `prebuild` hook is a lot like
|
|
`needsbuild` (but later), so it's called even if no trail
|
|
or trail member has actually been edited.
|
|
|
|
For it to be useful for `trail`, the `prebuild` hook has to run
|
|
after both pagespecs and sorting work. The other use case
|
|
I've seen for a similar hook was for Giuseppe Bilotta to
|
|
sort an inline-of-inlines by mtime of newest post, but that
|
|
can't be the same hook, because it has to run after pagespecs
|
|
work, but before sorting.
|
|
|
|
--[[smcv]]
|
|
|
|
> I've merged trail3-integrated, but not prebuild. I don't exactly dislike
|
|
> prebuild, but dunno that the hook prolieration is worth the minor cleanup
|
|
> it allows in trail. --[[Joey]]
|
|
|
|
>> Hmm, t/trail.t is failing several tests here. To reproduce, I build the
|
|
>> debian package from a clean state, or `rm -rf .t` between test runs. --[[Joey]]
|
|
|
|
<pre>
|
|
t/trail.t .................... 1/?
|
|
# Failed test at t/trail.t line 211.
|
|
# Failed test at t/trail.t line 213.
|
|
# Failed test at t/trail.t line 215.
|
|
# Failed test at t/trail.t line 217.
|
|
# Failed test at t/trail.t line 219.
|
|
# Failed test at t/trail.t line 221.
|
|
# Failed test at t/trail.t line 223.
|
|
# Failed test at t/trail.t line 225.
|
|
# Failed test at t/trail.t line 227.
|
|
# Failed test at t/trail.t line 229.
|
|
# Failed test at t/trail.t line 231.
|
|
</pre>
|
|
|
|
> Looking at the first of these, it expected "trail=sorting n=sorting/new p="
|
|
> but gets: "trail=sorting n=sorting/ancient p=sorting/new"
|
|
>
|
|
> Looking at the second failure, it expected "trail=sorting n=sorting/middle p=sorting/old$"
|
|
> but got: "trail=sorting n=sorting/old p=sorting/end"
|
|
>
|
|
> Perhaps a legitimate bug? --[[Joey]]
|
|
|
|
>> I saw this while developing, but couldn't reproduce it, and assumed
|
|
>> I'd failed to update `blib` before `make test`, or some such.
|
|
>> In fact it's a race condition, I think.
|
|
>>
|
|
>> The change and failure here is that `sorting.mdwn` is modified
|
|
>> to sort its trail in reverse order of title. Previously, it
|
|
>> was sorted by order of directives in the page, and secondarily
|
|
>> by whatever sort order each directive specified (e.g.
|
|
>> new, old and ancient were sorted by increasing age).
|
|
>> `old` appearing between `new` and `ancient`, and `new` appearing
|
|
>> between `end` and `old`, indicates that this re-sorting has not
|
|
>> actually taken effect, and the old sort order is still used.
|
|
>>
|
|
>> I believe this is because the system time (as an integer) remained
|
|
>> the same for the entire test, and mtimes as used in ikiwiki
|
|
>> only have a 1-second resolution. We can either fix this with
|
|
>> utime or sleep; I chose utime, since sleeping for 1 second would
|
|
>> slow down the test significantly. Please merge or cherry-pick
|
|
>> `smcv/trail-test` (there's only one commit). --[[smcv]]
|
|
|
|
----
|
|
|
|
[[!template id=gitbranch branch=smcv/ready/trail author=smcv]]
|
|
|
|
Some later changes to trail:
|
|
|
|
* Display the trail links at beginning/end of default `page.tmpl`
|
|
as suggested on IRC
|
|
* Improve CSS, particularly in blueview and goldtype themes
|
|
([example](http://blueview.hosted.pseudorandom.co.uk/posts/second_post/))
|
|
* Fix a possible bug regarding state deletion
|
|
|
|
--[[smcv]]
|
|
|
|
> Applied --[[Joey]]
|