sile-typesetter / sile

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

Noindent not taking effect after after chapter in manual? #2045

Closed alerque closed 3 weeks ago

alerque commented 3 weeks ago

Is losing noindent after chapter and section a new feature?

0.14.17 manual

image

0.15.0 manual

image

Originally posted by @Omikhleia in https://github.com/sile-typesetter/sile/issues/2044#issuecomment-2154283381

alerque commented 3 weeks ago

I would assume it's getting set in the wrong place. We had a bug that got squashed where the setting could sometimes leak to the N+2 full paragraph instead of applying to the next paragraph start. With that bug quashed now we're seeing the manual doing it wrong. At least that's my guess, I'll look into it.

alerque commented 3 weeks ago

This might be a case of fixing one bug surfaced more and reverting until we fix all of them might be the way forward. My paragraph setting handling in #2043 seems to have backfired. Trying to sort this out bumps into "HACK" and "FIXME" comments leading to #1699 and #1760.

alerque commented 3 weeks ago

And ... class:endPar() is getting called ... sometimes when it is not the end of a paragraph. And also not sometimes when it is the end of a paragraph.

alerque commented 3 weeks ago

And for good measure we're outputting parindent sometimes when we're not actually at the start of a paragraph. No wonder we can't figure out pushBack().

Omikhleia commented 3 weeks ago

reverting

Reverting the code change might actually be the best course of action. I know it's not easy, but take a deep breath. Trying to fix the SIL-language space and paragraph handling issues should not have significant impacts on the internal typesetter logic[^1] and class hooks[^2], unless approached with great caution.

For the record, as stated in the PR, #1760 was a very conservative approach, struggling with a viable non-breaking solution (#1699 = Jan. 27, #1718 = Feb. 23... and I only came up with a workaround, not even a true fix, after weeks of experiments and false starts = Apr. 6...)

Both #2041 and #2043 were too quickly merged and pushed to 0.15. I don't know what to say -- It's a narrow path -- Except that the backfire is certainly not what you intended for this release, which has lots of great things already. I am sure the intention was good, and I don't want you to be under pressure trying to keep these changes and fix them with another layer of workarounds at all costs.

[^1]: Well actually yes, there are deep issues. The fact that the typesetter assumes blank lines in the input to be paragraphs (https://github.com/sile-typesetter/sile/blob/6ad9b762c6cdd2f164c7e343c8a5b3ebbf13d717/typesetters/base.lua#L307) is a big concern to me: I am afraid that shouldn't be the case, under a clear separation of concerns... (and this patternsep thing is what verbatim changes, but verbatim then loses all blank lines...).

[^2]: "Settings" scopes are messy, as noted elsewhere. Parindent is problematic indeed, but maybe you'll realize on the way that hangIndent/hangAfter are heavily problematic too, and they are not alone. There's a reason why I never proposed upstream my changes to hangIndent/hangAfter for https://github.com/sile-typesetter/sile/discussions/1742#discussioncomment-6732830 -- while they are in silex.sile, I know it's another layer of temporary workarounds that needs greater caution too...)

alerque commented 3 weeks ago

Thanks for the feedback and encouragement. You're right this isn't how I wanted the release to go, but also note the release was borked because the distribution sources are missing a file, so it couldn't even be built properly. I'm correcting that with a patch release now, and also dealing with the other issues.

I also agree that it would have been better to hash out details of how and where paragraph calls were being made over a longer period. However my main consideration was this: We know the input grammar handling was borked, and we've known it for a long time. I'm quite confident the change in that regard is for the better and having fought to work around this on project after project I was unwilling to go for a whole new major release cycle without fixing that.

There was never any reason for these two things to be different, and it is absurd that we didn't fix it sooner:

\command{foo}

bar

vs.

\begin{command}foo\end{command}

bar

I did look into reverting #2043 but the more I messed with trying to make it the same as before the more edge cases were breaking. The original change in #2041 to normalize command vs. environment syntax handling on the grammar level just had too much impact not to actually fix some of the errors downstream of that. Removing goofy workarounds for the unexpected effects it was producing was a large part of that.

For better or worse I'm committed to the SIL grammar being fixed (and also now better matching XML handling) so that these forms are 100% equivalent and neither one unexpectedly gobbles up space in a way that processing commands and modules could not touch. The v0.15 has that as a starting point. I think I've fixed some of the other rough edges that change brought up, and we can continue to fix more in patch releases if we need to, but I am not really open to reverting the base premise about the grammar itself being incorrectly implemented.