1. It seems that caching was broken in
3adaf4e5fc, because I changed what key
was used to retrieve from the cache but not the key that was used to
store in the cache. I guess I hadn't noticed the performance
difference. It should be fixed now.
2. While making this fix, the test suite caught a bug in the new,
almost-fixed version that was caused by not including the preamble in
the key, and it only did so because the tests ran sequentially in the
same Emacs, making use of caching. If each test were independent and
ran with a clean cache, it wouldn't have caught the bug, and it would
undoubtedly have been quite a head-scratcher, or even a hair-puller,
at some future time.
3. And I would have pushed a version including that bug if I didn't
have a git pre-push hook that runs the tests, because somehow I missed
running the tests on that particular change.
So, lessons reinforced: tests are good; automated tests are better;
enforced tests are best.
4. Narrowed queries are now cached using point-min/max in the buffer.
This will be especially helpful for the WIP recursive queries feature.
I do wonder if the overhead of caching might be a drawback in some
cases, however some simple benchmarks of recursive queries that return
about 2,000 results shows a large improvement from caching, reducing
runtime from 3.12 to 0.24 seconds, so it's probably worth it, overall.
Planning-line-related predicates searched entire entries, which could
find lines that looked like planning lines but were not. For example, in an
entry talking about tiny.el, in a source code block, in Elisp comments
that had text resembling an Org planning line, that text would be
found by the regexp search, and the predicate would then attempt to
parse the match, which could fail.
Now the regexp search for planning-line-related predicates is bound by
the end of the line after the heading, which is the only place that
actual Org planning lines are supposed to be.
Sometimes org-element-context returns an element like:
(planning (:closed (timestamp (:type inactive ...))
:deadline (timestamp (:type active ...))
:scheduled (timestamp (:type active ...))
:begin 93706 ...))
Other times it returns just:
(timestamp (:type active ...))
Even with point in the same position, immediately after the deadline
timestamp. I don't know why, and it might even be a bug that's been
fixed in newer version of Org.
Anyway, this handles both.
Those pcase clauses looked nice and clean, but putting the logic in
the macrolet lets us use number arguments to the keywords, which is
useful.
Eventually I may want to unify this, or put it in the defpred macro.
