sile-typesetter / sile

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

Typesetter and package support for constructs spanning multiple lines #1977

Closed Omikhleia closed 8 months ago

Omikhleia commented 8 months ago

New approach to #1334 (with nicer user-facing API), based on https://github.com/Omikhleia/silex.sile/pull/10 -- See the latter for illustration and screenshot.

Caveat: It does further break the broken "pushback" logic (due to additional reboxing)

Omikhleia commented 8 months ago

N.B. Proposed on "develop" for 0.15 as the pdf changes there make it easier. The other changes (i.e. rules, color) were working on 0.14.16.

Is it me or GitHub doesn't notice the "Closes XXX" text in enumerations? ^^

alerque commented 8 months ago

This passes all the regression and unit tests, but it won't build the manual (which is why the CI build failure). At far as code review I liked most of what I saw so far. I'm a little concerned about exposing all the public methods on typesetter. If those actually need to be exposed to the end user and packages etc. that's fine, but if there is any reason to consider them an implementation detail that could be refactored without having to maintaine support for the way the previous methods worked I would mark them as private (underscore prefix or make them local functions in the module).

Omikhleia commented 8 months ago

(underscore prefix or make them local functions in the module)

I was thinking the same, and many other methods in the typesetters are kind of internal too. I am not fond of making them local, as I like the idea anyone can tinker with them, especially in derived classed, and was thinking of prefixes too. _xxx or internalXxx or privXxx ? (I'd like to convey the idea that they are more or less "protected" (in the C++ sense) and anyone tinkering with them does so at their own risks ^^)

alerque commented 8 months ago

I've already been using the convention (used in many other Lua projects) of prefixing functions and variables with _ if they are intended to be "internal". SILE has a bunch of them already, and the main point I'd make is that I don't have any qualms about refactoring them with breaking changes without marking the release as breaking or providing shims. In other words "tinker with this at your own risk". Lets make as many of the typesetter bits as we reasonable can this kind of "privateish" so we don't have to support old versions if we decide to refactor while still leaving things open to tinkering. Of course anything that obviously makes sense to be user or package facing can be public.

alerque commented 8 months ago

Rebase of old commits was just to tweak a commit message. The only change I actually made was the minor commit at the end to bludgeon CJK into cooperation.