Skip to content

Conversation

@RayBB
Copy link
Contributor

@RayBB RayBB commented Sep 9, 2025

Closes #11101

Copy link
Collaborator

@tarleb tarleb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with a good test :) Thanks!

I'm leaving this for @jgm to merge, as this will set the precedence on how pagebreaks are represented in the AST.

@jgm
Copy link
Owner

jgm commented Sep 9, 2025

If we are using a div here, we might consider adding wrapper="1", so that it can be "unwrapped" in formats that allow putting an attribute directly on a horizontal rule.

@tarleb does that make sense?

For the class name, should it be pagebreak or page-break?

Or should we go whole hog and add PageBreak to the AST?

@tarleb
Copy link
Collaborator

tarleb commented Sep 9, 2025

Ah right, page-break would be better. English orthography remains a challenge for me, even after all those years :D

I had forgotten about our wrapper convention. @RayBB, the LLM will probably be able to assist with that. If not, please ping me here.

@tarleb
Copy link
Collaborator

tarleb commented Sep 9, 2025

Or should we go whole hog and add PageBreak to the AST?

I'll probably have about a week of quiet time soon, in which I should be able to work on some of those bigger changes. I guess we'd want to bundle a couple of AST changes into a single release?

@jgm
Copy link
Owner

jgm commented Sep 9, 2025

Right, I guess in German you are used to jamming words together!

I'm not sure about the AST change. I don't like to rely on string typing, though. So if there are a significant number of formats that would support page breaks, and we're willing to put in the work to support them in readers and writers, it might be worth considering. AST changes are disruptive, though.

@RayBB
Copy link
Contributor Author

RayBB commented Sep 9, 2025

It seems that you both are still considering a different approach of adding this to the AST more broadly.

I'm not sure if that means you want to merge this PR in the mean time or just wait.

In any case, I renamed pagebreak to page-break and added wrapper=1 so I think your suggestions are addressed if you do want to merge this. @tarleb

PS: If you want to squash and merge that's fine. Or I can do that locally and push it up if necessary.

@tarleb
Copy link
Collaborator

tarleb commented Sep 9, 2025

Excellent, thanks! I think the plan is to merge this, we're just thinking out loud about possible alternatives and future changes.

@jgm jgm merged commit c93acab into jgm:main Sep 10, 2025
11 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse #pagebreak() in Typst

3 participants