Numbered equations #1821
Numbered equations #1821
Conversation
…o set a custom counter id
… to number equations
…, counter="equation]{...}'
packages/math/typesetter.lua
Outdated
| SILE.call("increment-counter", { id=counter } ) | ||
| SILE.call("math-counterstyle", { id=counter } ) | ||
| else | ||
| SILE.call("hbox") |
There was a problem hiding this comment.
I suppose you did that to cancel the fact that regular glues are ignored at an end of line?
If so, i think this empty hbox trick can be avoid, if the glue is made explicit (i.e. on line 149 above, use pushExplicitGlue)
There was a problem hiding this comment.
this being said, this approach does work well in ragged (left or right) environment. ❗
There was a problem hiding this comment.
Checking how LaTeX behaves in justified/raggedleft/raggedright/center - additionally nested in a block (quote)1
We should design a test scenario and check how we compare...
Footnotes
-
problematic with our current implementation of ragged environments (not nesting, not keeping parindent, so basically behaving differently, in a questionable way) ↩
There was a problem hiding this comment.
Thanks for the pushExplicitGlue suggestion -- I didn't knew that!
As a (really) side note, since you brought the ragged environments topic: when you suggested to use options to number equations in the display environment, I also wondered: what if we had an option to place the number on the left, or on the right side? Playing a bit, I saw that, surprisingly (to me), documents with direction=RTL already place the numbering on the left side (although the equation may be bugged -- eq. 1 is not centered, and parenthesis in equation 2 are wrong)
Since left numbered equations are not my use case, I opted to not add any option in this regard.
There was a problem hiding this comment.
So let's craft a regression test similar to my above LaTeX tests. That test eventually should be committed in tests/math-display-numbered.sil and has it's -b debug result also committed as tests/math-display-numbered.expected
Before any change:
After your changes as of c4db864
It breaks expectations with both numbered and unnumbered display math. That tells the importance of having (non-)regressing tests for features. 😸
What I would expect, at least, is:
Which I achieved hacking your proposal as follows:
(N.B. it could possibly be simplified, it basically re-implements or rather adapts a raggedright environment, respecting possible nesting)1
if mode == "display" then
SILE.typesetter:endline()
SILE.typesetter:pushExplicitVglue(SILE.settings:get("math.displayskip"))
SILE.settings:temporarily(function ()
local lskip = SILE.settings:get("document.lskip") or SILE.nodefactory.glue()
local rskip = SILE.settings:get("document.rskip") or SILE.nodefactory.glue()
SILE.settings:set("document.lskip", SILE.nodefactory.hfillglue(lskip.width.length))
SILE.settings:set("document.rskip", SILE.nodefactory.glue(rskip.width.length))
SILE.settings:set("document.parindent", SILE.nodefactory.glue())
SILE.settings:set("current.parindent", SILE.nodefactory.glue())
SILE.settings:set("typesetter.parfillskip", SILE.nodefactory.glue())
SILE.settings:set("document.spaceskip", SILE.length("1spc"))
SILE.typesetter:pushHorizontal(mbox)
SILE.typesetter:pushExplicitGlue(SILE.nodefactory.hfillglue())
if counter then
SILE.call("increment-counter", { id=counter } )
SILE.call("math:numberingstyle", { id=counter } )
end
SILE.typesetter:endline()
end)
SILE.typesetter:pushExplicitVglue(SILE.settings:get("math.displayskip"))
But the devil is in the details, and as you noted it probably doesn't work well in RTL...
Also, what happens if the equation is long and takes up most of the line so the fill and the number cannot fit? This would also call from a dedicated test.
Footnotes
-
The current implementation of
\raggedrightin SILE 0.14 is subpar, and I might suggest at some point to go for the re-implementation I made in my silex.sile module. Whatever, here for the purpose of display math, it's probably safer anyway not to depend from it. ↩
|
Sorry I haven't been able to jump in with PR review on this yet. I do appreciate the contribution and am pretty confident it will be worth merging. That being said there are a few unresolved comments from @Omikhleia that are worth resolving, and then there will be some commit message cleanup to happen before this is mergable. I am going to bump this one minor patch release cycle because it's holding up a patch release for stuff that's been in master for a while and I kind of need to be available in production. |
|
Still a bunch of unhandled review comments here that look like they make sense to me. Bumping again... |
|
So, I have made a few chances in this PR. |
Unless I am not fully awake (which is possible!), I don't see the changes here. Are you sure you pushed them?
A good practice (that we don't always follow, perhaps) is for the person who made a remark to mark it as resolved after having reviewed the applied changes. |
|
Live @Omikhleia I'd be happy to give this some more review, but I also don't see any new content. There was a merge commit pushed in the last 2 months but it seems to be all content I expect from a merge, not anything releated to this PR. Are you sure there are not more commits you have locally or even uncommitted stuff locally that needs to get pushed here? |
Enable numbering in mathml; Revert the definition of a '\equation' command; Fix documentation.
Using 'pushExplicitGlue', as suggested Renamed the numbering format to 'math:numberingstyle'
|
Another try. |
|
That's better, now there is something new to review. We'll have to fix the commit messages but don't worry about that yet, we can fix that after everything else is reviewed and ready. |
| @@ -344,6 +359,18 @@ Finally, here is a little secret. This notation: | |||
|
|
|||
| \noindent In other words, the notation using \code{&} and \code{\\\\} is only a syntactic sugar for a two-dimensional array constructed with braces. | |||
|
|
|||
| \paragraph{Numbered equations} | |||
| Equations can be numbered in display mode adding the options \autodoc:parameter{numbered} and \autodoc:parameter{counter}. | |||
| When \autodoc:parameter{numbered=true}, the equations are numbered using a default "equation" counter. | |||
There was a problem hiding this comment.
nitpicking: "example" --> “example” with curly typographical quote marks.
| Equations can be numbered in display mode adding the options \autodoc:parameter{numbered} and \autodoc:parameter{counter}. | ||
| When \autodoc:parameter{numbered=true}, the equations are numbered using a default "equation" counter. | ||
| A different counter may be set by using the option \autodoc:parameter{counter=id}, and this setting will also enable numbering. | ||
| The default numbering format is "(xx)", but this style may be overriden by defining a custom \autodoc:command{\math:numberingstyle} function, e.g. |
There was a problem hiding this comment.
"(xx)" --> I would suggest \autodoc:example{(xx)}` so that it is typeset in the example font (Cormorant Infant currently) as other examples of "rendered content". We could avoid the quote marks here, then: no need for double marking! (But if we want to keep them, then use typographical quote marks.
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
nit: Lots of extra blank lines inserted!
| The default numbering format is "(xx)", but this style may be overriden by defining a custom \autodoc:command{\math:numberingstyle} function, e.g. | ||
|
|
||
| \begin[type=autodoc:codeblock]{raw} | ||
| \define[command=math:numberingstyle]{(\show-counter[id="equation"])} |
There was a problem hiding this comment.
I am not fond of the example using a macro, because macros can't use options, so here this example would always use the "equation" counter regardless of what the user passed to the \math command. I'd suggest removing the example, and just expanding the description as follows (my changes in bold):
... this style may be overridden by defining a custom \autodoc:command{\math:numberingstyle} command. The counter id is passed as parameter to this hook.
(By the way, overridden takes two -d-, I think)
|
Looks quite good to me now - I marked most of my previous remarks as resolved and just opened a few new ones, mostly documentation-oriented. |
| \paragraph{Numbered equations} | ||
| Equations can be numbered in display mode adding the options \autodoc:parameter{numbered} and \autodoc:parameter{counter}. | ||
| When \autodoc:parameter{numbered=true}, the equations are numbered using a default "equation" counter. | ||
| A different counter may be set by using the option \autodoc:parameter{counter=id}, and this setting will also enable numbering. |
There was a problem hiding this comment.
Suggestion: \autodoc:parameter{counter=id} --> \autodoc:parameter{counter=<id>}
The angle brackets are replaced with nice brackets and the content is shown in a different color when autodoc.autodoc.highlighting is enabled. (And italic would possibly be used, although maybe the Hack Italic font is not installed in regular builds, IIRC)
|
Closing this PR (inactive for 7+ months) so as to move forward, see #2020 for an updated/curated version. |
With this PR, a new 'numbered' mode is added in math, in addition to 'display' and 'text'. Equations in this mode have the same appearance of the ones in the 'display' mode, except that a counter is added at the right end of the line.