From 1584b09708a81a96f52b20a29dfb36c04d6e71ab Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Fri, 12 Mar 2021 18:48:11 +0100
Subject: [PATCH] =?UTF-8?q?Fix=20pagebreak-in-box=20bug=20=E2=9C=94?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/exec/context.rs          |  54 ++++++++++++++++++++---------------
 src/library/page.rs          |  10 ++++---
 tests/ref/library/shapes.png | Bin 2510 -> 3266 bytes
 tests/typ/library/shapes.typ |  17 +++++++++++
 4 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/src/exec/context.rs b/src/exec/context.rs
index e10c28ff4..6a1c24167 100644
--- a/src/exec/context.rs
+++ b/src/exec/context.rs
@@ -23,7 +23,7 @@ pub struct ExecContext<'a> {
     /// The tree of finished page runs.
     tree: Tree,
     /// Metrics of the active page.
-    page: PageData,
+    page: Option<PageInfo>,
     /// The content of the active stack. This may be the top-level stack for the
     /// page or a lower one created by [`exec`](Self::exec).
     stack: NodeStack,
@@ -38,7 +38,7 @@ impl<'a> ExecContext<'a> {
             env,
             diags: DiagSet::new(),
             tree: Tree { runs: vec![] },
-            page: PageData::new(&state, Softness::Hard),
+            page: Some(PageInfo::new(&state, Softness::Hard)),
             stack: NodeStack::new(&state),
             par: NodePar::new(&state),
             state,
@@ -130,16 +130,18 @@ impl<'a> ExecContext<'a> {
 
     /// Execute a template and return the result as a stack node.
     pub fn exec(&mut self, template: &ValueTemplate) -> NodeStack {
-        let prev_par = mem::replace(&mut self.par, NodePar::new(&self.state));
-        let prev_stack = mem::replace(&mut self.stack, NodeStack::new(&self.state));
+        let page = self.page.take();
+        let stack = mem::replace(&mut self.stack, NodeStack::new(&self.state));
+        let par = mem::replace(&mut self.par, NodePar::new(&self.state));
 
         template.exec(self);
-        let stack = self.finish_stack();
+        let result = self.finish_stack();
 
-        self.par = prev_par;
-        self.stack = prev_stack;
+        self.page = page;
+        self.stack = stack;
+        self.par = par;
 
-        stack
+        result
     }
 
     /// Construct a text node from the given string based on the active text
@@ -190,24 +192,30 @@ impl<'a> ExecContext<'a> {
     }
 
     /// Finish the active page.
-    pub fn finish_page(&mut self, keep: bool, new_softnes: Softness) {
-        let stack = self.finish_stack();
-        let data = mem::replace(&mut self.page, PageData::new(&self.state, new_softnes));
-        if !stack.children.is_empty() || (keep && data.softness == Softness::Hard) {
-            self.tree.runs.push(NodePages {
-                size: data.size,
-                child: NodePad {
-                    padding: data.padding,
-                    child: stack.into(),
-                }
-                .into(),
-            });
+    pub fn finish_page(&mut self, keep: bool, new_softness: Softness, source: Span) {
+        if let Some(info) = &mut self.page {
+            let info = mem::replace(info, PageInfo::new(&self.state, new_softness));
+            let stack = self.finish_stack();
+
+            if !stack.children.is_empty() || (keep && info.softness == Softness::Hard) {
+                self.tree.runs.push(NodePages {
+                    size: info.size,
+                    child: NodePad {
+                        padding: info.padding,
+                        child: stack.into(),
+                    }
+                    .into(),
+                });
+            }
+        } else {
+            self.diag(error!(source, "cannot modify page from here"));
         }
     }
 
     /// Finish execution and return the created layout tree.
     pub fn finish(mut self) -> Pass<Tree> {
-        self.finish_page(true, Softness::Soft);
+        assert!(self.page.is_some());
+        self.finish_page(true, Softness::Soft, Span::default());
         Pass::new(self.tree, self.diags)
     }
 }
@@ -241,13 +249,13 @@ fn trim(nodes: &mut Vec<Node>) {
 }
 
 #[derive(Debug)]
-struct PageData {
+struct PageInfo {
     size: Size,
     padding: Sides<Linear>,
     softness: Softness,
 }
 
-impl PageData {
+impl PageInfo {
     fn new(state: &State, softness: Softness) -> Self {
         Self {
             size: state.page.size,
diff --git a/src/library/page.rs b/src/library/page.rs
index 963ab9137..20d7f0696 100644
--- a/src/library/page.rs
+++ b/src/library/page.rs
@@ -38,6 +38,7 @@ pub fn page(ctx: &mut EvalContext, args: &mut ValueArgs) -> Value {
     let main = args.get(ctx, "main-dir");
     let cross = args.get(ctx, "cross-dir");
     let body = args.find::<ValueTemplate>(ctx);
+    let span = args.span;
 
     Value::template("page", move |ctx| {
         let snapshot = ctx.state.clone();
@@ -83,20 +84,21 @@ pub fn page(ctx: &mut EvalContext, args: &mut ValueArgs) -> Value {
         }
 
         ctx.set_dirs(Gen::new(main, cross));
-        ctx.finish_page(false, Softness::Hard);
+        ctx.finish_page(false, Softness::Hard, span);
 
         if let Some(body) = &body {
             // TODO: Restrict body to a single page?
             body.exec(ctx);
             ctx.state = snapshot;
-            ctx.finish_page(true, Softness::Soft);
+            ctx.finish_page(true, Softness::Soft, span);
         }
     })
 }
 
 /// `pagebreak`: Start a new page.
-pub fn pagebreak(_: &mut EvalContext, _: &mut ValueArgs) -> Value {
+pub fn pagebreak(_: &mut EvalContext, args: &mut ValueArgs) -> Value {
+    let span = args.span;
     Value::template("pagebreak", move |ctx| {
-        ctx.finish_page(true, Softness::Hard);
+        ctx.finish_page(true, Softness::Hard, span);
     })
 }
diff --git a/tests/ref/library/shapes.png b/tests/ref/library/shapes.png
index 2804f8e96dc763cada90d727837431e94bc42c20..8ace6fd6d13c09a197748fef054189513ff764b7 100644
GIT binary patch
delta 2817
zcmZ8jdpMNa8gD=K2&p;c?4)ri5z(-7n|4C&9Z@djGL@;cw@ZcjVq`+b*Xe4Mp4=}Z
zx}Z&l+~zapxMW<@aL5?uLhhF_BQ%2<!x>83Ie&b=XT9J1uJyiaz3ch?R;zNY>N6F^
zZuyNHHf&hnp-@G8xBdifiL^g)+%1&F=ia!@>QZf-me|2P{yG(_gsU;58K6rVuKA?q
zCK+BnlisatpPV%(0TI;J*wzr$f+J<r!?9oH=4$pV%<pM^H0vCBNAJ4VecPIt&$fZI
zHL!X>e3o<aZ#$5-70HnYHhl+hbtrS2(h<_mpEe^os{w&Deny45QjmW%(Jp*w#()3#
zkb`cyS-qooD7GNHYTgXG8HJ`bG!rtoS(TdoU8YdIBkT5E_{zE+rk0D-lN}lwM=il%
z(fpX6<D<vSLRgSLnLR!Hi+a_s_ORZJtnt(gqYm0E*gQ=Q=A`jOqUrYb_WH2!Jo40d
zP?|^0%1>PW7UJMnPKElIwmNI?4(K5jQ5uPTwfVYu>=OPS&kTsc5f2_rrdnXb4jnB&
zOm<!l6%E2UX_Mv`)FyIu|9#Xt*h%Hawq3gLaa{hWyCyXyD_I<xqLH4{SXh~v=$#&r
z1`?YBr%}Rs>z$8Fpd$`2Blo^43kR_CalPl$$roX{&TY=Z`aiKRuRdDY{X^wU0?)be
z@kyGgdd7r>JN}TvBi+X9;fs2fM;&xG%R=~R%fxM&R;NDE>gj?z{aycr`fbXrer+VV
z1Ii)jJ70x+0Swk6e<@KsW?D~j1i<m!s?f#x@kR}uAgVv~^|N;n_Sry}8}csvy{u7z
z`^vYk19?DZ$^f5d*Iga&cdu&F1(?{c)!rM?3loU%V(Efctkj)zofspSjz?edSz1_V
z%NAoKUV)JF{*oKj3y75yjPmm|uVL8+9{^TO2^wTP=ek12L&5{86Zh&kiEIR~st_>Y
z_vQ_JA)CBxO1{-oGsi%5b<hYYPcz9CTF;;22iok$w$Y69o>pk;4;x^|3nMn39&1Z;
zIuoDP%J52VO3c$2Y3@9vUV@LU@=#cu(jMD{*)F!`dgmV@^@(~9H=}kAwYYaZAFeQ!
zg865V_a9y`*3%kKBEQm3V_lv&^)ov=+D^MQz9+u`JX4KJvN2#};}4V|?Pg+0FQQJz
z)IX84FB)Zx>?Yi|G1jYO*->8ykb;DVc4?QmPZnM-_AzaV?%>OP40txwaz^`u8SItt
zNRK;`c!Fn$oTB(X<r`<qldfq*dHbEH1vyc5+58Mk<}oq`q`0|6clU{fl;&XOX5-t%
zqe~Gi<wLtqXZh^Vu^oxEETH1uZzV-N*$3Kgw*8o2Dtr<yh_g)h<~8Bx0}>6`+h>hS
zTRu`!K9)N2aT38V=h)kV++XO$M6m-N0Dmyc=!elli8OaNq0`J)7<ehSW;_6-A7lI;
z+fyu!tiG{)u;uMbmL_xGE$!kA;K^_-NIxG_-4w%m?BYF{A$Q~!^Ar4pg{2e9%jddx
zDx>YS$BB!s6(b$m(OMQ$0aPhAmjkP01A@^#P`%b*8$Hj<v)A41KiQe}i*G5J7tX?}
z+I{&Ow>p}kP78X+^3|q9izu*~`4{ThkP5c~?il)<Xyjvzpz%11gqUEfDt!w?0~-+O
zpoBI!q`$Li^n?bHg4{iUeZy^7nl<nzq4#ihZf~EC;Y1v;h)rZt?2FBs=pQi1xp!TG
zY{#rGDyjh0{yg}lxQv*ujA7>up#TAn@UhFbfpN8)zJsl0!UUZfBD(3x(Q4_M&Bcr)
z^tqFO*39TY-UG`VlD_<;*9|uP=|F+Xw#B?Rf=k8e7hLBqqj2R8a>h7b%DYSOd&G+7
za6sj*&%qt}=WMm#`PZGQ3*9bxz4&T&&lc^@T!mUcRhL9X@@b#r9m)je($ABMW^v)-
zi-z34j`DAc?eU;DJfd@aa^a8mg5O@{lDg*(R2z=79=xiwbDf7h@b-f?a6_(G!*GX`
zdu%XSwofdT$~caYo+m#B%luTEKRHGJE%r+4u<Q891@mW@Glr91o`EJHcI8Lyu%t$2
zqN2`0TPX)wvFSm9vPs(vB_%A09`k&u)30szBZA8>>I3}=B&vULnDyhrjM(yU!XAS;
zj*U1d^5Q|s>r|fmCCSYIj%S^3s><x1Ydy<bfC*o=+05^RrjT0;n4*e=h0isv$%^J=
zCw5tM${4LtGa{8g;SkF8Txh?g%U$F4^?LA~|A+Qb`0;Itc<?*B$W%eb-cXQ4ywH+t
z<Y^hUP-y)rjz4juk5?j<K5F}Fkyh*u^o_+!WtMgIn*;0-UxTo0Qy(}tQk<F-kjl26
zo10s1UF<SP;+JXFVEFP@M2|Ui4mMXER$zTJ3&9>98G%NDa&1ho#G^;Pv>oFqL$IBX
zmKtGt456!{$Y${CZB{V?(2~HGv!|BDZ>ptVO#(1jiHK36>zrW?&ZtxG;Dmp3WB<by
z#gP?W-Y=89{}Hwo#gloz@ue*82b&7Y;MY+<^blN|Hw01eHdeT*+Hb7;ZRKxbC?njz
z7g%wuWwR^c+Lf$|-WsfU_N|9+1+wuW$_vtyr`*|7RxTHPz<ImXa%2_9Yb)%EM<^(O
ze5hiT+E+5IRK5o5r2PMOxYFIVygv?Mee(6nu`+j6QdXfuAK8(#F3Cf<!uyyAeMl1C
zCLgb%pdqUWCi@gYh+ScyaM4mxVhFoSa(S;Jt&Cj$*L%`lz&06fHSN)OHeqa0|LiXx
z29MSDdTivR_3yla*4|C9bKbmk-wsk^M)Kq#-*Uu0K88vGQWlgu7)9Wb29hdp+^~Lz
z2KbhVymIdE1!4l>b;jLldeXx^Lft?6xU4wIXR`4Q0Nj>95QJzvQEc09{Am;PkP?D9
z$5PW%rpJ4uo<`^<C_y=t=KQ|#n>p}}K4wm~9&+Vo48(a|`#2?Yxep&4<swn>sjY$j
zN$eM_ER0e@8JcONr_)u8fb0bX;(@pP?)iyh7pTGrBjsl4u>*g0SK|RBGk$UK3Y(s9
zD!S@cTPDV^OCPaEaq;gu{WeVcjsFoadiuoO1%@N~d;)`frB2%1#`oRs){<f3!nm%p
z!-#*Jl7Uf*+J7KP8VS0W`uI==d2z%BL(A8Cel0VbAhO)L1D4+<aa--8>^iJ<CnkUz
zl(}VHkgN-y!COYm2j54qWUr{lMCs1@q7l*C`P!mZ^o%sQoFKsp#kY+H-+Bf{qtTp6
z$f_{VpGb7<uNJ97D9YA?$SD&56BK{mgu@yc$r$KrT{5SU5@+GOIP(fpmmEZ<^hnV+
zlDlzSy0ae_`v&(DRnX-s?TEVnS_lZq|4?5BYYx9P=J7i@WFA0Oi`P_8|9(Xj^!cuo
km5aOfpj;UZ%vie_>ZHHB5;!V*>Ta;NaXL}(+m*Zj1uetGxc~qF

delta 2221
zcmaJ@dsI?;7Vde|jN3}5Oc`}jl3vrpymHlmYJ5axrlwq@tnM;RlcR`&`8sp0(TLR2
zYo;W<6iibRAG}g4D>Ecl8j=E05^5$sGBHqez?*lR`D51lt+T(q&v*83?QgHO_jzLV
z%HoRUayOI33l=Q66F#V@S_fGOnq0zo>^YLqt?t5Or7x`h`60ZxGxGShv!;zpk_ZH)
zyaal`lGYQL3&~s!KkSMb`!Y47Xy54|a;5sd==poGxol$D1DhRdh`|n&i~bfCjtdb_
zGGm%DV;p3L(|>?#Dn>qr{3q5l4?D<A5TMLJ9_YQ6{OOT_W^jxT0TXCK0r3P2%#X(#
z!*UM3ic7;CmwD*mXT{?^5?Kxk_z4GxCz6V^sOJ>`K>gf0u>@gACd+|Yp^<_Iz8Suq
z(XVW6Y#lM+sBU~MCsE+DO^2BgVVsi5Ik0ek2uwL1s!Pqp)r0bPNVi?PcAbDTBeRi3
zj+CXxltIy)dt0MALh<r+H;j%%d0Np6m{r)`=--aXQeeUNiJ7<xxucm-HSHElcxOlV
zRX3q;B2&)eisX*eDth7*>UaUs4cr-8biYJ;M55Y3LSeRk^+Q|H9XllN+V-;#b&7n7
z8xxqi9y^fwtjk>uSDJ~*!_6%L$P>xrGnOK=6gEn1Np0cLGYREX+E;i3#huQeDpPa5
z`i|F^HT1*vxe+-BvvWs(E?8k;NLSB9cZ0DvV9ICX(~Hs#V(I!+<1N-4FP_gE`63vr
znfp@<{@65jU^F5Pmn+K&NEz*aol6T@4-y1;PG|!AXDdgpSUlUd|83EeKU$_fu>RCU
zP&$Mko6oPLUG$h7JBQ$^ZC*OpXPyc#R0MGJTYf)Ntt?I-i)prTlBAeXveKk$HCa4Q
zYc5LwQFJ6oko#HgkE@HG9~!=gV<q!A(vdx26d=SmR}Gg-4@%u3e;rD6{EK$L1^%sp
zA73*PW6x0e;_z34gs2hLzJ7YT=MmId5ZH{qQZD#nvE?=>u1UM}rY2P@Tg|I@`I|n&
z^Pn(lxY#ae(M+^?O&Y^QpA9(qBg<0NdcPZ$TV7zxYmmaY2!2E*U<&Er=6knq1qXK%
z;zG0iIe`uJfwq)5c7jX!^WnhZBF5|e@ezubgA+Z2kAA#DUar3A+W$=3fTf*<nfDS{
zBazFxQTq_x8__@1C)5pmtIv<K616{!AukT=q*kBaV1BxA>FY2HMIZuqU%U>XAa=;A
z@;9D4<5|5M7U*Ce=2aZ<wu#v0170bT`bMv|wAY-_kMmlb$SaC9&yW*J?<9LKck_r<
z-c?u6h2F$wq_~o-sh90%E@>`b4?flG`j#jd`~Z`C3^j#k9P3)VWAomm0$0o2LdHEc
zNnsNf*>^htxMJz`R37!Op(mAiCZTSCB=C-e`6CKcobtNmH(qsGl}=q_f+Q)xk=CNG
zg{l12H?f<^Ft#hd=9>33(mitOP>3~hn}^rXsar%2I;p<@5Gz-8G<a<=L#!+{Uxmm6
z_|92(UeL*Pa#Vd_Z~i+xVxl&`OJuJ+LCb5?Yk*SvR()`5BW<#<SKI?VZd=d1y-^a`
zBHQM+*^4|m6{N59155rk3YT~1Ge$Nc8uKu<Cal=8ZO0NteHm=#vsZPr6l!2?u3C7v
z+1gXQipD?T{6<wof@<4XQ8k6X#o#<Vyd|+?xGsxeLLTiYjw-wVmPAyzSOWse$5eO|
z9${L(GK5cnfBuC=3S{TfCW(|*7cVbW_}ilOb)I1W9lWf4NK<TzjY(a90E+|g$QZc&
z%u(izQ`r$2%NI?iL`y}LJwqD?wdM4I{v1_nAzyL&biR-acOctXxO{&8o)54Kq-Kxr
zIG9HTdBGk?E|y#zwbvXNWp>(i?B0uX+Ve|D`tP`<Qe4TNhw%2UZ{!Eq(yK?9ZL0Q)
zm);dYAfl4qWr{vitI@O1u=d_@aDRYiOvPiQAsf1joOv^c_myeZ+^RPn?0X#2>Y=&g
z;_vVr1*zrI0Kd9!^b6V5H4INOxaQIlp|g7q=^6vsC2VhX`Z!RjQ>#NFd$x$&u=v&c
zuiM4#M1L~>$ft;QiF$wSjc`L~zA}T7=q2D#yBsw!arD<_fEM@8$GtJx(@lk25>Yiw
zYG;kzP3}To=iM#C+htvzq+|t$X?K}eYi5>8;;YB3wIv=O%NoB(MP(JO;g<Bs^7ri(
z9hy>mN8Qv|TZ2dw<M?oZIW>?3mzy9AGsL(L3?q6L8&`xOVuIj<>o^Pbjn?nep&z`s
zAJU<~1t=ikri@Lgg!wGn#wjf>j2+x8&DKS2PZ&7}p4<dgX%5@1{NemEJM{KEA7b_S
zhyDyU1&BsF<w1y(eJ8hnbMNCYc%L$EPLFGv{t?EG09dvwY*CV%w%hYXvBW-3AHXYS
z@KWx|Oq^fi!COY7PSUR=c5UH=Ylhd)blMdN8C=Q=+o}6@)4Ke|dcBEZWHx}~gQLp6
zHoKWe$9Gp}&2F^W5{wJXxb2K%^S`S!Ry-S?tM)r}s1_>T{9O+4Pi^`+yc8dVTr#Hu
z_q!l-)*#FroU=!w5oK7TvkEYr?}Ryg0cw=f7?-BRTWIx=z$}db`VykafG(d~v`vs)
zsi`f<Q4GxHFnnOV^v`_r{rT8<*L;6I{(tZM?#fCx&L{f?w9hOmwOF_z>)3^Tv4-E0
O1sKmm9+i7fru`ez@QZi=

diff --git a/tests/typ/library/shapes.typ b/tests/typ/library/shapes.typ
index ceaa0148c..efb68edfd 100644
--- a/tests/typ/library/shapes.typ
+++ b/tests/typ/library/shapes.typ
@@ -21,3 +21,20 @@ Sometimes there is no box.
 #box(width: 0.5in, height: 10pt, color: #D6CD67)
 #box(width: 0.5in, height: 10pt, color: #EDD466)
 #box(width: 0.5in, height: 10pt, color: #E3BE62)
+
+---
+// Make sure that you can't do page related stuff in a box.
+A
+#box[
+    B
+    // Error: 16 cannot modify page from here
+    #pagebreak()
+
+    // Error: 11-15 cannot modify page from here
+    #page("a4")
+]
+C
+
+// No consequences from the page("A4") call here.
+#pagebreak()
+D