Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SILE.scratch.headers shouldn't be declared only in twoside? #2086

Open
jodros opened this issue Jul 22, 2024 · 8 comments
Open

SILE.scratch.headers shouldn't be declared only in twoside? #2086

jodros opened this issue Jul 22, 2024 · 8 comments
Labels
question Ask for advice or investigate solutions

Comments

@jodros
Copy link
Contributor

jodros commented Jul 22, 2024

Just now I got:

Error: runtime error: /usr/local/share/sile/packages/twoside/init.lua:123: attempt to index field 'headers' (a nil value)

Well, since this table is only used by twoside dependents, it should be declared there instead of having to declare whenever one creates a new class/package that relies on it.

@Omikhleia
Copy link
Member

Minimum working example and test?

'Cause we can go on monkey patching, sure, but you do realize "headers" are broken to their core, right? (#1842, #1291)

@Omikhleia
Copy link
Member

Also: "scratch" variables ought to get killed :)

@jodros
Copy link
Contributor Author

jodros commented Jul 22, 2024

Also: "scratch" variables ought to get killed :)

Wow, so the refactoring is bigger than I thought... What's going to be in its place? Can't we do anything with them for now?

'Cause we can go on monkey patching, sure, but you do realize "headers" are broken to their core, right? (#1842, #1291)

I'll take a look over this.

@Omikhleia Omikhleia added the question Ask for advice or investigate solutions label Jul 22, 2024
@Omikhleia
Copy link
Member

Omikhleia commented Jul 22, 2024

Also: "scratch" variables ought to get killed :)

Package A or class C tinkering with scratch variables from package B breaks all separation of concerns and deeply ties the implementations, making the scratch variable a part of the public API of a package, that any "equivalent" replacement would have to honor (beyond just providing commands). So the concept is somewhat defective by design.

'Cause we can go on monkey patching, sure, but you do realize "headers" are broken to their core, right? (#1842, #1291)

I'll take a look over this.

Aside note: I have (partly) solved some of my concerns with my resilient.book class and my resilient.headers package -- though I know they still have some of above-mentioned issues regarding proper separation of concerns.

@jodros
Copy link
Contributor Author

jodros commented Jul 22, 2024

breaks all separation of concerns and deeply ties the implementations

Now I got it, but what's going to replace all those scratch variables?

@Omikhleia
Copy link
Member

Now I got it, but what's going to replace all those scratch variables?

I'm afraid it's a case by case basis, due to the different use cases for them, and the lack of global styling approach in the core distribution.

@Omikhleia
Copy link
Member

See also #1417

In many cases indeed, private members of the package might be a better approach.

@alerque
Copy link
Member

alerque commented Aug 23, 2024

Yes, all SILE.scratch variables need to die. All of them. 100% no mercy.

Taking steps towards that by only defining variables where they are used is good, and it's only a short step from there to properly scoped variables that go with the class/package/document/whatever.

We still have some setting scope stuff to deal with, but class and package scopes that have getters and setters on anything that needs external access and internal local variables for everything else covers most use cases I've run into.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Ask for advice or investigate solutions
Projects
None yet
Development

No branches or pull requests

3 participants