Refactor document classes using Penlight model #1065
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alerque
force-pushed
the
penlight-classes
branch
from
ffab63a to
b2264f1
Compare
|
Wow, talk about timing. Penlight 1.9.0 finally landed with a fix more multiple inheritance that this work is going to need! |
alerque
changed the title
alerque
force-pushed
the
penlight-classes
branch
2 times, most recently
from
fc4458c to
6963475
Compare
alerque
force-pushed
the
penlight-classes
branch
from
6963475 to
d6a68bb
Compare
Versions of Penlight prior to 1.9.0 suffered from issues with class inheritance beyond one level and with scoping issues in calls to self:super(). I had worked around these in much of the previous usage by setting the base class manually and calling it directly in its own scope rather than using the automated mechanism. Since those workarounds are no longer needed, this reverts to as original a generic a syntax as possible. In the process it was revealed that my workaround was actually much uglier under the hood than I realized. Rather than inheriting from ClassC→ClassB→ClassA my workaround actually caused an artificial extra degree of separation ClassC→plain→ClassB→plain→ClassA where each plain object was a new class with inherited methods but not actual class properties. The final syntax here is verbose and ugly, but it's correct Penlight class usage. I'm sure I'll eventually settle on a more elegant syntax for the job, but for now working properly is more important.
alerque
force-pushed
the
penlight-classes
branch
from
d6a68bb to
9d460a8
Compare
BREAKING CHANGE: This changes the way classes are represented as Lua objects and the mechanism used for inheritance. While shims will be in place to catch most cases that use old syntax it is not possible to grantee 100% API compatibility. If you have classes that do anything remotely fancy (i.e. not just copy/paste from SILE examples) they may or may not work at all; and even if they do they should be updated to explicitly use the new API.
Having these in a Texlike typesetter at this particular location introduces a race condition because the typesetter may not be initialized. We could check for it, but we then limit how the class can be used and in some use cases it is useful to initialize a class *then* attach a typesetter to the class's default frame.
alerque
force-pushed
the
penlight-classes
branch
2 times, most recently
from
145a49d to
270b1ee
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is long overdue work towards removing
stdlib, in this case replacing the class model (the object oriented programming concept) used in SILE classes with Penlight classes.In some ways this is a follow up to #1064 that started some much-needed code organization so that there would even be something regular to replace. See also #1062 and wiki documentation of this project.