sile-typesetter / sile

The SILE Typesetter — Simon’s Improved Layout Engine
https://sile-typesetter.org
MIT License
1.65k stars 98 forks source link

fix: no error '! Couldn't find frame ID folio' anymore #1943

Open jodros opened 9 months ago

jodros commented 9 months ago

This "error" always annoyed me when I create framesets without a folio frame and I suppose other people do it was well.

What you think?

jodros commented 9 months ago

Well, even though it seemed to have worked sometimes, I noticed maybe it hasn't to do with this part lol, at least not at all, because it still give this error sometimes when using -d frames. But I just can't find where its declaration is in the code.

jodros commented 9 months ago

Now I finally got rid of it when running with -d frames.

The warning was pointing to where the endpage hook is declared, so I created a global variable in order to use the pcall check within the hook function.

BTW I'd like to know whether this is a good idea or not, and whether there are other ways to do this work without using global...

Omikhleia commented 9 months ago

This "error" always annoyed me when I create framesets without a folio frame and I suppose other people do it was well.

It's perhaps a documentation problem? Silencing an error (absence of folio frame when the class expects it) is probably wrong, moreover using a global variable.

jodros commented 9 months ago

Silencing an error (absence of folio frame when the class expects it) is probably wrong.

My point it that the class shouldn't require a folio frame, when users create framesets without folio, they shouldn't be warned because of that. What is wrong in having no folio in the frameset?

Omikhleia commented 9 months ago

What is wrong in having no folio in the frameset?

Nothing per se, but folios can be disabled with nofolios (or nofoliothispage), which can be invoked when switching to a frameset that doesn't need one anyway. And any "advanced enough" class would likely have sections with folios and sections without -- so it needs to do it properly anyway, and I am unsure of the benefit of hiding an error.

jodros commented 9 months ago

and whether there are other ways to do this work without using global...

🤦 It's impressive how simply getting enough sleep can make our brains function a bit better...

Now isFolioFrame is declared as local before the function.

jodros commented 9 months ago

I am unsure of the benefit of hiding an error.

I understand your point, but please note that the only "error" this PR would hide it not an actual error!

The current implementation of this package assumes every frame set as having a folio, what isn't true. It only would be an error unfairly hidden if the user wish to have a folio frame, but simple forgot to declare it, and received no warning, does this make any sense?

But the opposite can happen as happened with me, but why have to explicitly call nofolios when the package could just test whether there is the folio frame, and only call outputFolio() if so?