Closed lkndl closed 10 months ago
As mentioned on #5, I have some tests and a small testing script locally that I wanted to push first. Most importantly, regression tests.
I've taken a short look and the code seems reasonable to me, but quite a bit more complicated than it has to be. The fix I have locally simply adds an after/before bound to the selector in core.get-adjacent
with a higher level. Especially the extra use of core.is-redundant
seems problematic, this one should only check if a relevant heading is visible, such that the header would be redundant.
The "testing framework" is written nushell and a bit of python, if you have both of those you could test it yourself after rebasing, otherwise I'll test it locally.
I was thinking about something like on this branch, because it doesn't have to be manually set for the easy cases, see the scoped-search
test reference images, I believe that is what you specified.
nice! But i don't think the core.is-redundant
was so far off. The test case below (adapted with everything below = Second
a bit different) fails on page 7 with a redundant header referencing section 2.2
right above the new chapter heading 3 Third
.
= First chapter
#lorem(10)
== First chapter first section
#lorem(150)
== First chapter second section
#lorem(20)
= Second
#lorem(30)
== Second section zero
=== Second subsection one
#lorem(90)
== Second section one
#lorem(25)
= Third
#lorem(20)
== Third section one
#lorem(180)
= Fourth
#lorem(200)
That's true, the redundancy has to check for this too. It seems that this complicates the API more than I hoped though, the next heading should use a different selector to account for its own and higher levels. Maybe I do away with selectors in general, they only seem to cause trouble here.
Ok, so I've done some thinking and want to outline that.
The main goal of hydra is to be easy to use in the most common cases, such that, adding a simple named argument is enough:
#hydra(binding: left) // assume 2 pages are visible with left-hand binding
#hydra(paper: "a7") // use a7 paper margins for the next-on-top redundancy check
#hydra(fallback-next: true) // display the next heading if it is on top (this is foten undesired, but some people explicitly asked for it)
As shown in #5, or more specifically (your prior example), this doesn't always work well when selectors are involved or headings are skipped. The search is not yet scoped, and the redundancy doesn't account for the scope either.
The main idea of allowing selectors was familiarity and providing a simple API. Would a user only want to consider chapters, they could use #hydra(sel: heading.where(level: 1)
and it would work as expected. But it was also meant to allow users to add their own custom elements (people usually get around the lack of custom elements with figure(kind: "blah blah")
).
I think want to restrict the sel
API to a subset of selectors which are always turned into a (elem, function)
pair, so that the redundancy check works without any problems. But I fear this removes flexibility that I wanted to retain for external packages to add their own element types. (Some more context can be found here, look for independently numbered headings like legal paragraphs)
If only heading selectors are allowed, we can't add custom heading-like elements to show. If we do allow external ones, it's very hard to know how they relate in terms of scoping. And this likely needs a rework of the general inner workings.
So I've been working v0.3.0
since #5 was opened, and the internal API has changed quite a bit by now. It's already fixed on that branch, so I'll close this. Thank you for putting in the work and for taking the time to test my branch to iron out issues with the implementation!
Fixes #5 . Hope you don't mind?
was that important or just a
!= none
check? i don't quite getfallback-next
yet. cheerio!