Skip to content

First round of DRYing common render code.#1679

Merged
rocky merged 2 commits intomasterfrom
DRY-render-code
Feb 5, 2026
Merged

First round of DRYing common render code.#1679
rocky merged 2 commits intomasterfrom
DRY-render-code

Conversation

@rocky
Copy link
Member

@rocky rocky commented Feb 5, 2026

Actually, before DRYing, I ran into a little issue here that the boxes field always only contains a single box, and calling lookup routines for "boxes" is confusing or misleading.

So before doing the DRYing, going in small steps, I think it's best to get this out of the way.

@rocky rocky marked this pull request as draft February 5, 2026 16:09
Since that is what it is.
@rocky rocky requested a review from mmatera February 5, 2026 17:13
@rocky rocky marked this pull request as ready for review February 5, 2026 17:15
@rocky
Copy link
Member Author

rocky commented Feb 5, 2026

@mmatera: your thoughts?

@mmatera
Copy link
Contributor

mmatera commented Feb 5, 2026

@mmatera: your thoughts?

If I get this right, this is just renaming. As far as I can see, I think is OK. LGTM

@rocky
Copy link
Member Author

rocky commented Feb 5, 2026

@mmatera: your thoughts?

If I get this right, this is just renaming. As far as I can see, I think is OK. LGTM

There are also some additional slight changes to mathics.core.formatter that will assist in DRYing code. And there's a redundant hard-coded character translation table removed. (The original copy should be removed or reduced as well, but that's for another time.

@rocky rocky merged commit d93c279 into master Feb 5, 2026
21 checks passed
@rocky rocky deleted the DRY-render-code branch February 5, 2026 19:07
@mmatera
Copy link
Contributor

mmatera commented Feb 5, 2026

Just a detail: these methods are called in Mathics Django and several of the Mathics3 modules. We need to adjust them accordingly.

@rocky
Copy link
Member Author

rocky commented Feb 5, 2026

Just a detail: these methods are called in Mathics Django and several of the Mathics3 modules. We need to adjust them accordingly.

Noted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants