>>>> To fix that I'll need to pass a reference to that array into pagespec_makeperl.
>>>> I think I can then do the same thing to $params{specFuncs}. -- [[Will]]
+>>>>> You're right -- I did not think the recursive case through.
+>>>>> --[[Joey]]
+
> * Seems that the only reason `match_glob` has to check for `~` is
> because when a named spec appears in a pagespec, it is translated
> to `match_glob("~foo")`. If, instead, `pagespec_makeperl` checked
>>>> and
- define(aStar, a*) and link(aStar)
+ define(aStar, a*) and link(~aStar)
>>>> In the first case, we want the pagespec to match any page that links to a page matching the glob.
>>>> In the second case, we want the pagespec to match any page that links to a page matching the named spec.
>>>> match_link() was already doing existential part. The patches to this code were simply to remove the `lc()`
>>>> call from the named pagespec name. Can that `lc` be removed entirely? -- [[Will]]
+>>>>> I think we could get rid of it. `bestlink` will lc it itself
+>>>>> if the uppercase version does not exist; `match_glob` matches
+>>>>> insensitively.
+>>>>> --[[Joey]]
+
> * Generally, the need to modify `match_*` functions so that they
> check for and handle named pagespecs seems suboptimal, if
> only because there might be others people may want to use named
>>>> Yes, and it can do that in just three lines of code. But if we automatically check for named pagespecs all the time we
>>>> potentially break any matching function that doesn't accept pages, or wants to use multiple arguments.
+>>>>> 3 lines of code, plus the functions called become part of the API,
+>>>>> don't forget about that..
+>>>>>
+>>>>> Yes, I think that is the tradeoff, the question is whether to export
+>>>>> the additional complexity needed for that flexability.
+>>>>>
+>>>>> I'd be suprised if multiple argument pagespecs become necessary..
+>>>>> with the exception of this patch there has been no need for them yet.
+>>>>>
+>>>>> There are lots of pagespecs that take data other than pages,
+>>>>> indeed, that's really the common case. So far, none of them
+>>>>> seem likely to take data that starts with a `~`. Perhaps
+>>>>> the thing to do would be to check if `~foo` is a known,
+>>>>> named pagespec, and if not, just pass it through unchanged.
+>>>>> Then there's little room for ambiguity, and this also allows
+>>>>> pagespecs like `glob(~foo*)` to match the literal page `~foo`.
+>>>>> (It will make pagespec_merge even harder tho.. see below.)
+>>>>> --[[Joey]]
+
+>>>>>> I've already used multi-argument pagespec match functions in
+>>>>>> my data plugin. It is used for having different types of links. If
+>>>>>> you want to have multiple types of links, then the match function
+>>>>>> for them needs to take both the link name and the link type.
+>>>>>> I'm trying to think of a way we could have both - automatically
+>>>>>> handle the existential case unless the function indicates somehow
+>>>>>> that it'll do it itself. Any ideas? -- [[Will]]
+
> * I need to check if your trick to avoid infinite recursion
> works if there are two named specs that recursively
> call one-another. I suspect it does, but will test this
>>>>> Looks like its bracketed in the code anyway... -- [[Will]]
+>>>> Perhaps the thing to do is to have a `clear_defines()`
+>>>> function, then merging `A` and `B` yields `(A) or (clear_defines() and (B))`
+>>>> That would deal with both the cases where `A` and `B` differently
+>>>> define `~foo` as well as with the case where `A` defines `~foo` while
+>>>> `B` uses it to refer to a literal page.
+>>>> --[[Joey]]
+
+>>>>> I don't think this will work with the new patch, and I don't think it was needed with the old one.
+>>>>> Under the old patch, pagespec_makeperl() generated a string of unevaluated, self-contained, perl
+>>>>> code. When a new named pagespec was defined, a recursive call was made to get the perl code
+>>>>> for the pagespec, and then that code was used to add something like `$params{specFuncs}->{name} = sub {recursive code} and `
+>>>>> to the result of the calling function. This means that at pagespec testing time, when this code is executed, the
+>>>>> specFuncs hash is built up as the pagespec is checked. In the case of the 'or' used above, later redefinitions of
+>>>>> a named pagespec would have redefined the specFunc at the right time. It should have just worked. However...
+
+>>>>> Since my original patch, you started using closures for security reasons (and I can see the case for that). Unfortunately this
+>>>>> means that the generated perl code is no longer self-contained - it needs to be evaluated in the same closure it was generated
+>>>>> so that it has access to the data array. To make this work with the recursive call I had two options: a) make the data array a
+>>>>> reference that I pass around through the pagespec_makeperl() functions and have available when the code is finally evaluated
+>>>>> in pagespec_translate(), or b) make sure that each pagespec is evaluated in its correct closure and a perl function is returned, not a
+>>>>> string containing unevaluated perl code.
+
+>>>>> I went with option b). I did it in such a way that the hash of specfuncs is built up at translation time, not at execution time. This
+>>>>> means that with the new code you can call specfuncs that get defined out of order:
+
+ ~test and define(~test, blah)
+
+>>>>> but it also means that using a simple 'or' to join two pagespecs wont work. If you do something like this:
+
+ ~test and define(~test, foo) and define(~test, baz)
+
+>>>>> then the last definition (baz) takes precedence.
+>>>>> In the process of writing this I think I've come up with a way to change this back the way it was, still using closures. -- [[Will]]
+
>> Secondly, it seems that there are two types of dependency, and ikiwiki
>> currently only handles one of them. The first type is "Rebuild this
>> page when any of these other pages changes" - ikiwiki handles this.
>>>>> are inlined, the previous type handles a change in the content of any of those pages. Shortcut does not need this type of
>>>>> dependency. Most of the places that use `add_depends()` seem to need this type of dependency rather than the first type.
+>>>>>> Note that inline and map currently achieve the second type of dependency by
+>>>>>> explicitly calling `add_depends` for each page the displayed.
+>>>>>> If any of those pages are removed, the regular pagespec would not
+>>>>>> match them -- since they're gone. However, the explicit dependency
+>>>>>> on them does cause them to match. It's an ugly corner I'd like to
+>>>>>> get rid of. --[[Joey]]
+
>>>>> Implementation Details: The first type of dependency can be handled very similarly to the current
>>>>> dependency system. You just need to keep a list of pages that the content depends upon. You could
>>>>> keep that list as a pagespec, but if you do this you might want to check that the pagespec doesn't change,
>>>>> possibly by adding a dependency of the second type along with the dependency of the first type.
+>>>>>> An example of the current system not tracking enough data is
+>>>>>> where A inlines B which inlines C. A change to C will cause B to
+>>>>>> rebuild, but A will not "notice" that B has implicitly changed.
+>>>>>> That example suggests it might be fixable without explicitly storing
+>>>>>> data, by causing a rebuild of B to be treated as a change to B.
+>>>>>> --[[Joey]]
+
>>>>> The second type of dependency is a little more tricky. For each page, we'd need a list of pagespecs that
>>>>> the page depended on, and for each pagespec you'd want to store the list of pages that currently match it.
>>>>> On refresh, you'd need to check each pagespec to see if the set of pages that match it has changed, and if
Patch updated to use closures rather than inline generated code for named pagespecs. Also includes some new use of ErrorReason where appropriate. -- [[Will]]
+> * Perl really doesn't need forward declarations, honest!
+
+>> It complained (warning, not error) when I didn't use the forward declaration. :(
+
+> * I have doubts about memoizing the anonymous sub created by
+> `pagespec_translate`.
+
+>> This is there explicitly to make sure that runtime is polynomial and not exponential.
+
+> * Think where you wrote `+{}` you can just write `{}`
+
+>> Possibly :) -- [[Will]]
+
----
diff --git a/IkiWiki.pm b/IkiWiki.pm