From 16d8a6a028bbfbbd808d255182e5ca3aa327b193 Mon Sep 17 00:00:00 2001 From: Omikhleia Date: Sun, 26 Mar 2023 16:47:59 +0200 Subject: [PATCH 1/4] fix(typesetter): New typesetter instances shall not reset settings --- typesetters/base.lua | 142 ++++++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 64 deletions(-) diff --git a/typesetters/base.lua b/typesetters/base.lua index b2751dfbb..9fce37e79 100644 --- a/typesetters/base.lua +++ b/typesetters/base.lua @@ -1,3 +1,81 @@ +--- SILE typesetter (default/base) class. +-- +-- @copyright License: MIT +-- @module typesetters.base +-- + +-- Settings common to any typesetter instance. +-- These shouldn't be re-declared and overwritten/reset in the typesetter +-- constructor (see issue https://github.com/sile-typesetter/sile/issues/1708). +-- On the other hand, it's fairly acceptable to have them made global: +-- Any derived typesetter, whatever its implementation, should likely provide +-- some logic for them (= widows, orphans, spacing, etc.) + +SILE.settings:declare({ + parameter = "typesetter.widowpenalty", + type = "integer", + default = 3000, + help = "Penalty to be applied to widow lines (at the start of a paragraph)" +}) + +SILE.settings:declare({ + parameter = "typesetter.parseppattern", + type = "string or integer", + default = "\r?\n[\r\n]+", + help = "Lua pattern used to separate paragraphs" +}) + +SILE.settings:declare({ + parameter = "typesetter.obeyspaces", + type = "boolean or nil", + default = nil, + help = "Whether to ignore paragraph initial spaces" +}) + +SILE.settings:declare({ + parameter = "typesetter.orphanpenalty", + type = "integer", + default = 3000, + help = "Penalty to be applied to orphan lines (at the end of a paragraph)" +}) + +SILE.settings:declare({ + parameter = "typesetter.parfillskip", + type = "glue", + default = SILE.nodefactory.glue("0pt plus 10000pt"), + help = "Glue added at the end of a paragraph" +}) + +SILE.settings:declare({ + parameter = "document.letterspaceglue", + type = "glue or nil", + default = nil, + help = "Glue added between tokens" +}) + +SILE.settings:declare({ + parameter = "typesetter.underfulltolerance", + type = "length or nil", + default = SILE.length("1em"), + help = "Amount a page can be underfull without warning" +}) + +SILE.settings:declare({ + parameter = "typesetter.overfulltolerance", + type = "length or nil", + default = SILE.length("5pt"), + help = "Amount a page can be overfull without warning" +}) + +SILE.settings:declare({ + parameter = "typesetter.breakwidth", + type = "measurement or nil", + default = nil, + help = "Width to break lines at" +}) + +-- Typesetter base class + local typesetter = pl.class() typesetter.type = "typesetter" typesetter._name = "base" @@ -39,70 +117,6 @@ function typesetter:init (frame) end function typesetter:_init (frame) - - SILE.settings:declare({ - parameter = "typesetter.widowpenalty", - type = "integer", - default = 3000, - help = "Penalty to be applied to widow lines (at the start of a paragraph)" - }) - - SILE.settings:declare({ - parameter = "typesetter.parseppattern", - type = "string or integer", - default = "\r?\n[\r\n]+", - help = "Lua pattern used to separate paragraphs" - }) - - SILE.settings:declare({ - parameter = "typesetter.obeyspaces", - type = "boolean or nil", - default = nil, - help = "Whether to ignore paragraph initial spaces" - }) - - SILE.settings:declare({ - parameter = "typesetter.orphanpenalty", - type = "integer", - default = 3000, - help = "Penalty to be applied to orphan lines (at the end of a paragraph)" - }) - - SILE.settings:declare({ - parameter = "typesetter.parfillskip", - type = "glue", - default = SILE.nodefactory.glue("0pt plus 10000pt"), - help = "Glue added at the end of a paragraph" - }) - - SILE.settings:declare({ - parameter = "document.letterspaceglue", - type = "glue or nil", - default = nil, - help = "Glue added between tokens" - }) - - SILE.settings:declare({ - parameter = "typesetter.underfulltolerance", - type = "length or nil", - default = SILE.length("1em"), - help = "Amount a page can be underfull without warning" - }) - - SILE.settings:declare({ - parameter = "typesetter.overfulltolerance", - type = "length or nil", - default = SILE.length("5pt"), - help = "Amount a page can be overfull without warning" - }) - - SILE.settings:declare({ - parameter = "typesetter.breakwidth", - type = "measurement or nil", - default = nil, - help = "Width to break lines at" - }) - self.hooks = {} self.breadcrumbs = SU.breadcrumbs() From 596a7a0aeb5d87a1c3c89eaa7f9788e05823cfa5 Mon Sep 17 00:00:00 2001 From: Omikhleia Date: Sun, 26 Mar 2023 16:51:01 +0200 Subject: [PATCH 2/4] test(typesetter): Add test for settings after new typesetter instantiation Test for regression #1708 --- tests/bug-1708.expected | 66 +++++++++++++++++++++++++++++++++++++++++ tests/bug-1708.sil | 12 ++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/bug-1708.expected create mode 100644 tests/bug-1708.sil diff --git a/tests/bug-1708.expected b/tests/bug-1708.expected new file mode 100644 index 000000000..0972fa188 --- /dev/null +++ b/tests/bug-1708.expected @@ -0,0 +1,66 @@ +Set paper size 209.7637818 297.6377985 +Begin page +Mx 17.4104 +My 41.1471 +Set font Gentium Plus;10;400;;normal;;;LTR +T 47 w=4.7607 (L) +Mx 27.1711 +T 82 w=5.0293 (o) +Mx 37.2004 +T 85 w=3.9600 (r) +Mx 46.1604 +T 72 w=4.6191 (e) +Mx 55.7795 +T 80 w=8.0322 (m) +Mx 66.4616 +T 76 w=2.7100 (i) +Mx 74.1715 +T 83 w=5.2979 (p) +Mx 84.4694 +T 86 w=3.8623 (s) +Mx 93.3317 +T 88 w=5.2979 (u) +Mx 103.6295 +T 80 w=8.0322 (m) +Mx 111.6618 +My 37.9684 +Set font Gentium Plus;6.81152;400;;normal;;;LTR +T 20 w=3.1962 (1) +Mx 17.4104 +My 53.1471 +Set font Gentium Plus;10;400;;normal;;;LTR +T 47 w=4.7607 (L) +Mx 27.1711 +T 82 w=5.0293 (o) +Mx 37.2004 +T 85 w=3.9600 (r) +Mx 46.1604 +T 72 w=4.6191 (e) +Mx 55.7795 +T 80 w=8.0322 (m) +Mx 66.4619 +T 76 w=2.7100 (i) +Mx 74.1719 +T 83 w=5.2979 (p) +Mx 84.4697 +T 86 w=3.8623 (s) +Mx 93.3320 +T 88 w=5.2979 (u) +Mx 103.6299 +T 80 w=8.0322 (m) +Mx 17.4104 +My 247.7565 +Set font Gentium Plus;9;400;;normal;;;LTR +T 20 w=4.2231 (1) +Mx 21.6335 +T 17 w=2.0610 (.) +Mx 41.6946 +T 87 w=3.0981 (t) +Mx 49.7927 +T 72 w=4.1572 (e) +Mx 58.9499 +T 86 w=3.4761 (s) +Mx 67.4260 +T 87 w=3.0981 (t) +End page +Finish diff --git a/tests/bug-1708.sil b/tests/bug-1708.sil new file mode 100644 index 000000000..cf6d14b1d --- /dev/null +++ b/tests/bug-1708.sil @@ -0,0 +1,12 @@ +\begin[papersize=a7,class=book]{document} +\neverindent +\nofolios +\set[parameter=document.letterspaceglue, value=5pt] +% Bug 1708: New typesetter instance (as e.g. done via a footnote) +% shouldn't reset settings from the global scope. + +Lorem ipsum\footnote{test} + +Lorem ipsum% Should still have letterspacing at 5pt + +\end{document} \ No newline at end of file From cd9c85efbd9592a417f6e679af06956b7c210969 Mon Sep 17 00:00:00 2001 From: Omikhleia Date: Sun, 26 Mar 2023 17:08:29 +0200 Subject: [PATCH 3/4] test(packages): Fix bidi non-regression test setting restoration --- tests/bidi.sil | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/bidi.sil b/tests/bidi.sil index 1d6ab9bc2..1e26ebff0 100644 --- a/tests/bidi.sil +++ b/tests/bidi.sil @@ -37,4 +37,6 @@ One two אבג three four My hovercraft is full of eels. הרחפת שלי מלאה בצלופחים + +\set[parameter=typesetter.breakwidth]% restore default before folio is output (see #1708) \end{document} From fd9bf8112c871d306d79e877790aedbe2349ff1a Mon Sep 17 00:00:00 2001 From: Caleb Maclennan Date: Wed, 29 Mar 2023 11:01:01 +0300 Subject: [PATCH 4/4] refactor(typesetters): Move setting declarations back to init but protect previously declared ones --- core/settings.lua | 4 ++ typesetters/base.lua | 145 ++++++++++++++++++++++--------------------- 2 files changed, 79 insertions(+), 70 deletions(-) diff --git a/core/settings.lua b/core/settings.lua index caf1da966..a456d6ff2 100644 --- a/core/settings.lua +++ b/core/settings.lua @@ -111,6 +111,10 @@ function settings:declare (spec) if spec.name then SU.deprecated("'name' argument of SILE.settings:declare", "'parameter' argument of SILE.settings:declare", "0.10.10", "0.11.0") end + if self.declarations[spec.parameter] then + SU.debug("settings", "Attempt to re-declare setting: " .. spec.parameter) + return + end self.declarations[spec.parameter] = spec self:set(spec.parameter, spec.default, true) end diff --git a/typesetters/base.lua b/typesetters/base.lua index 9fce37e79..4b9eefd5a 100644 --- a/typesetters/base.lua +++ b/typesetters/base.lua @@ -4,76 +4,6 @@ -- @module typesetters.base -- --- Settings common to any typesetter instance. --- These shouldn't be re-declared and overwritten/reset in the typesetter --- constructor (see issue https://github.com/sile-typesetter/sile/issues/1708). --- On the other hand, it's fairly acceptable to have them made global: --- Any derived typesetter, whatever its implementation, should likely provide --- some logic for them (= widows, orphans, spacing, etc.) - -SILE.settings:declare({ - parameter = "typesetter.widowpenalty", - type = "integer", - default = 3000, - help = "Penalty to be applied to widow lines (at the start of a paragraph)" -}) - -SILE.settings:declare({ - parameter = "typesetter.parseppattern", - type = "string or integer", - default = "\r?\n[\r\n]+", - help = "Lua pattern used to separate paragraphs" -}) - -SILE.settings:declare({ - parameter = "typesetter.obeyspaces", - type = "boolean or nil", - default = nil, - help = "Whether to ignore paragraph initial spaces" -}) - -SILE.settings:declare({ - parameter = "typesetter.orphanpenalty", - type = "integer", - default = 3000, - help = "Penalty to be applied to orphan lines (at the end of a paragraph)" -}) - -SILE.settings:declare({ - parameter = "typesetter.parfillskip", - type = "glue", - default = SILE.nodefactory.glue("0pt plus 10000pt"), - help = "Glue added at the end of a paragraph" -}) - -SILE.settings:declare({ - parameter = "document.letterspaceglue", - type = "glue or nil", - default = nil, - help = "Glue added between tokens" -}) - -SILE.settings:declare({ - parameter = "typesetter.underfulltolerance", - type = "length or nil", - default = SILE.length("1em"), - help = "Amount a page can be underfull without warning" -}) - -SILE.settings:declare({ - parameter = "typesetter.overfulltolerance", - type = "length or nil", - default = SILE.length("5pt"), - help = "Amount a page can be overfull without warning" -}) - -SILE.settings:declare({ - parameter = "typesetter.breakwidth", - type = "measurement or nil", - default = nil, - help = "Width to break lines at" -}) - -- Typesetter base class local typesetter = pl.class() @@ -117,6 +47,7 @@ function typesetter:init (frame) end function typesetter:_init (frame) + self:declareSettings() self.hooks = {} self.breadcrumbs = SU.breadcrumbs() @@ -129,6 +60,80 @@ function typesetter:_init (frame) return self end +function typesetter.declareSettings(_) + + -- Settings common to any typesetter instance. + -- These shouldn't be re-declared and overwritten/reset in the typesetter + -- constructor (see issue https://github.com/sile-typesetter/sile/issues/1708). + -- On the other hand, it's fairly acceptable to have them made global: + -- Any derived typesetter, whatever its implementation, should likely provide + -- some logic for them (= widows, orphans, spacing, etc.) + + SILE.settings:declare({ + parameter = "typesetter.widowpenalty", + type = "integer", + default = 3000, + help = "Penalty to be applied to widow lines (at the start of a paragraph)" + }) + + SILE.settings:declare({ + parameter = "typesetter.parseppattern", + type = "string or integer", + default = "\r?\n[\r\n]+", + help = "Lua pattern used to separate paragraphs" + }) + + SILE.settings:declare({ + parameter = "typesetter.obeyspaces", + type = "boolean or nil", + default = nil, + help = "Whether to ignore paragraph initial spaces" + }) + + SILE.settings:declare({ + parameter = "typesetter.orphanpenalty", + type = "integer", + default = 3000, + help = "Penalty to be applied to orphan lines (at the end of a paragraph)" + }) + + SILE.settings:declare({ + parameter = "typesetter.parfillskip", + type = "glue", + default = SILE.nodefactory.glue("0pt plus 10000pt"), + help = "Glue added at the end of a paragraph" + }) + + SILE.settings:declare({ + parameter = "document.letterspaceglue", + type = "glue or nil", + default = nil, + help = "Glue added between tokens" + }) + + SILE.settings:declare({ + parameter = "typesetter.underfulltolerance", + type = "length or nil", + default = SILE.length("1em"), + help = "Amount a page can be underfull without warning" + }) + + SILE.settings:declare({ + parameter = "typesetter.overfulltolerance", + type = "length or nil", + default = SILE.length("5pt"), + help = "Amount a page can be overfull without warning" + }) + + SILE.settings:declare({ + parameter = "typesetter.breakwidth", + type = "measurement or nil", + default = nil, + help = "Width to break lines at" + }) + +end + function typesetter:initState () self.state = { nodes = {},