-
Notifications
You must be signed in to change notification settings - Fork 44
Exclude headers in divs from outline #830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
||
// compute restricted ranges (ignore headings in these ranges) | ||
const isWithinIgnoredRange = isWithinRange(tokens, token => isCallout(token) || isTheorem(token) || isProof(token)); | ||
const isWithinIgnoredRange = isWithinRange(tokens, isDiv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me make things fun (of the type 3 variety).
What should we do about conditional content divs? (to say nothing of arbitrary Lua filters...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior of conditional content divs is that the div itself will never actually appear in the resulting document. But if the conditions are met, then the contents will be included in the document, and headers inside will "move one level up".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You always find and share the most interesting case! 😅
Level 3 indeed... How can we handle those indeed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well gosh. I could probably hack together some checks for that in this code... but should I?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vezwork No, I don't think that's the best move. Let's merge this as is and talk about a redesign for later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've realized that this PR doesn't actually address the regression. There is a deeper issue that probably is best resolved by deferring outline stuff to the CLI.
Fixes #829edit: this doesn't fix the more subtle issue I added to the bottom of #829. That seems to be a parser issue...
Some headers inside divs were already being excluded, but the exclusion rules were too conservative. The exclusions checked if the header was inside a callout (a div with a class that starts with
callout-
), a Theorem (a div with a theorem-specific id), or a Proof (a div with a proof-specific id). Now it excludes all headers inside any div.