From 62f315a4fcbf60ce86f57fd3bfc2d8de8810607c Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Fri, 3 Feb 2023 22:46:39 -0800
Subject: [PATCH 1/4] Permit Max Column for Row Breaks
Fix #3143. Page break was dropped. Difference between good and bad was the use of attribute `max` in `brk` tag in the good spreadsheet. However, `max` was *not* required in a similar spreadsheet. So the reason for the problem isn't completely explained. Nevertheless, it can't really hurt to capture the `max` value on read (if present) and generate it on write. This resolves the issue. User is also permitted to specify max column when setting a row break programatically. I am not yet in position to document when that might be a good idea.
---
src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php | 3 ++-
src/PhpSpreadsheet/Worksheet/Worksheet.php | 17 ++++++++++++-
src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 4 +++
.../Reader/Xlsx/RowBreakTest.php | 24 ++++++++++++++++++
tests/data/Reader/XLSX/issue.3143a.xlsx | Bin 0 -> 10003 bytes
5 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
create mode 100644 tests/data/Reader/XLSX/issue.3143a.xlsx
diff --git a/src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php b/src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php
index fb55cb2653..ab57548847 100644
--- a/src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php
+++ b/src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php
@@ -151,8 +151,9 @@ private function pageBreaks(SimpleXMLElement $xmlSheet, Worksheet $worksheet): v
private function rowBreaks(SimpleXMLElement $xmlSheet, Worksheet $worksheet): void
{
foreach ($xmlSheet->rowBreaks->brk as $brk) {
+ $rowBreakMax = isset($brk['max']) ? ((int) $brk['max']) : -1;
if ($brk['man']) {
- $worksheet->setBreak("A{$brk['id']}", Worksheet::BREAK_ROW);
+ $worksheet->setBreak("A{$brk['id']}", Worksheet::BREAK_ROW, $rowBreakMax);
}
}
}
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
index cd9b8ac884..cb444ac5a0 100644
--- a/src/PhpSpreadsheet/Worksheet/Worksheet.php
+++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -194,6 +194,13 @@ class Worksheet implements IComparable
*/
private $breaks = [];
+ /**
+ * Collection of max column values for row breaks.
+ *
+ * @var int[]
+ */
+ private $rowBreaksMax = [];
+
/**
* Collection of merged cell ranges.
*
@@ -1748,7 +1755,7 @@ public function duplicateConditionalStyle(array $styles, $range = '')
*
* @return $this
*/
- public function setBreak($coordinate, $break)
+ public function setBreak($coordinate, $break, int $rowMax = -1)
{
$cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate));
@@ -1758,6 +1765,9 @@ public function setBreak($coordinate, $break)
}
} else {
$this->breaks[$cellAddress] = $break;
+ if ($break === self::BREAK_ROW) {
+ $this->rowBreaksMax[$cellAddress] = $rowMax;
+ }
}
return $this;
@@ -1792,6 +1802,11 @@ public function getBreaks()
return $this->breaks;
}
+ public function getRowBreakMax(string $cellAddress): int
+ {
+ return $this->rowBreaksMax[$cellAddress] ?? -1;
+ }
+
/**
* Set merge on a cell range.
*
diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
index 0f3c5f753c..7ced3ed23f 100644
--- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
+++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
@@ -1026,6 +1026,10 @@ private function writeBreaks(XMLWriter $objWriter, PhpspreadsheetWorksheet $work
$objWriter->startElement('brk');
$objWriter->writeAttribute('id', $coords[1]);
$objWriter->writeAttribute('man', '1');
+ $rowBreakMax = $worksheet->getRowBreakMax($cell);
+ if ($rowBreakMax >= 0) {
+ $objWriter->writeAttribute('max', "$rowBreakMax");
+ }
$objWriter->endElement();
}
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
new file mode 100644
index 0000000000..3c13589746
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
@@ -0,0 +1,24 @@
+load($file);
+ $worksheet = $spreadsheet->getActiveSheet();
+ $writer = new XlsxWriter($spreadsheet);
+ $writerWorksheet = new XlsxWriter\Worksheet($writer);
+ $data = $writerWorksheet->writeWorksheet($worksheet, []);
+ $expected = '';
+ self::assertStringContainsString($expected, $data);
+ $spreadsheet->disconnectWorksheets();
+ }
+}
diff --git a/tests/data/Reader/XLSX/issue.3143a.xlsx b/tests/data/Reader/XLSX/issue.3143a.xlsx
new file mode 100644
index 0000000000000000000000000000000000000000..d90eb29f1648ac44476255d1e4cd6ad62bfe1667
GIT binary patch
literal 10003
zcmeHtg-O}BX(k@yn)>c(OLM8;D0?+{f05w3I&u7pU0RTWo0RRX9
z=!m+{>};JtwoV4mA@(4!9=n^3HPuICM8bW)`Pl_j)aH`rXOibh%H;QUuvt+
z8Df48FK|2%zL8Cn^2y5A3NMQceUeUuy|;%tT7jlmJ#EZxN%Mn-{=t)SjK}LOhsv80
zuprTA^bO2wd59NN+rr(g+K38F&P2xiZocYlB=Ykw>g(0GA8XMSC4tE`*<;SanFvc%
zYsxPv<+gf82-EdS%!vh_QHojQBjng{Q(`?10OFN!5gt;#_Cn>LovZDs=Ei87c6)2M
zEkjhBvRqi{Y;Y~;GaMPFnXWQk>RMyD5`lS?06yB`>B=q$X6w-b?I@oCZ{3JMZ%^!4
zvyL8m4{lJUaz!3M8vM|~0eBX3fce^8{9`h30pbSm;8=_A;r_#`7@T)w{QMEg(|~wa
zqlfPc1x~P#y|H$lIhs$MhLK&sW^m?GK8jO*i#xy}y@lJ^j~_??)xVi(g(fHMF&uk}
z@TZt?Gc|AoS%W#)f8PIRp8v%*`Io8ZM=7baabgAR$zBI`osG{$;z=mDip$nfKZknB
z&Ei&sXVQ_*H8GIkJty%+lJRVSUiZw*359QUQJ*gH=NOW%qfO!AzJ`?QHI8uV0gXC&c%7ufjq*TT15*3>5#B$^uLPc4d#
zQym3pAO@z^%}0_+O$0Y@*_4yJ)MSVS35^5V>>&mpK&Z-Q_n7wpU}g=;s=t+Nvsb7
zfsceVwO3{4K;D+qbiDj~Oy!cENs`g5ytSsiCw}D#`C4BGq}w|TI@jmjzk^O8x(}uw
z6R;J*0E=J(i+uU*uon>sm(b`9iV+<-Sl9#SFNnvjI>(UPE4wl!wp5C(OF1O3JZx2V
z&@!oX(prAjE@ZA4Lu;afG1o7{jb~b#GqK{SNhvEpYYw=ELkf?iZA3b&S31>BHw?h_6Qn9fijV&A;+M;jC&
z_UjU0_>*~L1(_GILie3@i&meCks@uaxq&wegfYD^ig<71@a^XH`?J-MO9fkK1v
zR}l?;R;tx5{kL1yhLJDRIg0P=Bwik8G<@Wq6i`EEJlLaWNpkOM-?>d8*(^E!;Y(qW
z7sk`v=LS
z%JFD#y^D3LmFbdTFf&(qG|~Rf;TkCnzc6j0Jo8hK%`_iVPDW+JvZj_DVwMU0xA?ZjPX_hN34KVW?u9720X3pT
zo_*dWD4j9;{@EW;HlENpeBgJ>m~mOz8W_>r(C9sOs^5>w7L-E|juWO`CeLm(N33X^
z%+uchA^}Zu2J+HRm8BE+$H|vy+C1<&P14QvOBCtRz3NGK)3s(kG7lCZ7p+(#Na
zC%S>t%@b~<|1@83k(l@+xY^diV;l(p9RY5>e}uWe8u5QZ9s-<^!kxf>_m!`%
zq|gp$WSF;soUTbO_wm0vanS5&ZsVeNmov>gqJ9iLULa+z)q6Riz=32J0NLyBbUAy1
zvxG!&(!!Dxau4|pf%Tpsie3LvKN7m}ZUKdl3<|-$ovm%yE+$SQn4m@^s*5$1kbmu*
zk)Aj(^AnBaam`2okL3WTGq)vph`a~x@_Wlyye|DTb_#sTtE#9Hnaoyy)I6IxMNRX8
zP!1mv1LcIUA%NkHF>03c7O7M8!}@D8CCGqPOXelkDj7_HBJYXt+W;r3z%Cg|Po^mF
z>c@mS&Amk@iK`x_rkO+5-s3&9mF~{*1^7@VZ;nF;VOQ1KcYT`E`XwZtU%8}?y{$YiF>`2m(S7;ST-Bscre`r4ecwt1;B>ZiP0Hu*Y-kBm$i#&y%s7lDIR6QrTT
z)$xWVDUXj1d63!;!KqpAkIQ78@8v>oAo}wHnal`{KkpPe`-B$
zoFzM<453UHy-maCYQwTu<~76%i4bC}l+K-m=+?{=Y^)ulI`Mo7m*$h)7^UP%V0Col
zqY~{0_nidBbFdXRw~5X^ushH7edLjd;q`T(S#^AW={zZQ800`RPTIYmc8vTJ*T248
zC_=&3R~1XD${2#6Bmq@L@a2uh5$M54{?<%DDM~#0T-yWtUdo)i4do0ZE3rv#n=@?a
zpbcZTF$JGztUpg_DT)H$*7(uQ7Rjd&XBT(Bz@P#%jkU*TJ-kvugxW=mUr@}@ms12@
zScFiKxN(+JJ@Zx@i}9i6No{J!oTCpw#FcaDQeLM!A{WhnKvqmx30-y1!5AyD3~W?i
zy~t49MR!d=ZGr|4jdH40wWnFVT;sDbN>#khPK7ocm8+!e%^k&*@&4ePR2g=${!noJ
zuRcDz7Rc}tZmgYDKg$ij4A#jUWCP;(b^m3nJKDOTqj$=`*qwnXu%f0s)&J%4)
z>www}^btC`r;IRSV!8Qnpm&teNw%u2f&qqyRYnXdiTQ>C<)sE9(B_i%O%}0xRz>pm
zr!3H9vD?ZX8F>o$c(ZT#BGlD*i`TSVd@+4!Uzl2rTRMim
zdX=MipF{urJoCt0L7KmVqKm;8v*KE|=c`r;qcmq&`%~UF;5Dz2kn}x@i*o+84Wi?Y
z@|R4pmS2e8x1Zf3^qn?3?!-;s*)o-oo@y!{oBzxyMdMFGq}EvEw5-_P{0_GlgZjZo
zg&J2s^WMPVjvcq4J=uj!S=sZ9)%fEygL?bMph|9PX1*>GN2HthZV=*fTS#iO3a(Z(
zsyNZuG|LT%*s4IG!QSL$MXU(6SSEfxPsNuB?II^%EGw^(BAmH~H$^&?`vJFGiT%5(
z9<24K*_Xagt__D!ICR{w{=yv(KkV8ucyDd}^}|d}-vP+MYNu$AEpMTk*Xnr!Jmw=;EBAj-U9f|w&^{em+DSmGY*xBzfLhRq06AUh
z9=(c4oBwgL>5Hba_anK#KvUashA|=0k)!d);hvr7(e34G-9y|a`OhN34iKqKXnt*@*$-DP%)cryqG6jg07YP@6YaX6E&!h<
z4mm`u>Y1~F^aFc4?;Om@mP$-JM?>O=`yC=;q1Bk?wwKJ{%!l;E)^z{n=7kZ&+J!vuHgobYF4
z*}F3o)^UvcBu}S|_#_VcEFR0z5L^ZG`w>dbKY66LuV=Q0cY@_0*4<&hKsSYP1`Ga%
z8VR#dPo>(5f9l$Pc+cz<`f3Q+${uHNG-W)=%P_FB(f3))a?en4n2W?@XYRb+NIbBg
zHEmtm;GLIdWeH0OJFVc0aj*}{sgqur@_RwamrQjLpRIj`>p}_I%f#fu&L8!=<2kc9
zFUEk?H#G$Wc)Z<_w?w-he%2_&7&4-B^^Bgd!Y>_QKVhA{sQspOt^YcrrLq3z)73rG
zebQb)NmHd=K#&f>3h=R{s$uoU1IhgTb(*bL(yf#abVaOC7h0o
zSgSXAoV&29=uEwLWfk)|FC<>(?CnxGTN*IK$$*`49=~>*_FK4_keB$}yAk1TO-%a-
zEtB~zri-7*EQli>UogENhWeQekuYzYtv@%-``ntug+NX#obR4{?q@san%G%qo$Mme
z3mP9+)MJe7>r_~@B&&VSEj76XIkU~yKYRAMwzrl?th+H7zr(Z=Uo#N@WR*&C5hT44
zn-X$gDeyjaHk3FIMEIf*Fsh~I{lXSke@qujltpf8-XRVV(Q=Sa>h9gh`<85(#rHHi
zkr+UK%8hg=%NJWf8vj`<{?oLTL6z0+oV@!Fr$^Wqo9mEhKx5o4;dZUNzru@+RAf|wrtoD1WZ?1sh!o`@!iCTfs
z>qs-GnNI#kWhK6Z#-?ZU#mu6TY%uk$iM=tJ_1d+lZ7Q{t&+RPAbCdjdSfhvCtfMhs
zwt4dT1xYKE1i$z+?@QYFE@-{>8zr!FKlGjL&eue1FD)DkF52ts*R^r6C+*5aI|~Fe
z`{U5hYJ-ub4Bj9mjG$V-7vzTFFDCJ=XRR)<`R}Id0_*(MX?Gs_SgJDfw7OIb;j~%S
zP{t`VD6dkGv2us>8oeDo3fmi}fH!i>lb%9%qu69WGF1-g6KZ{~`J$!3olXt8mo$8A-&
z`7kORt8~#V=|Rv;?HsgNdBL12(wlS-4i*!`RVEM9M!=b69s9orVn~LUbrw9)Qbz><
zu>T0eU?+$*2>g>SXKC74&2Zw~icERpU4HLzS!M-R_Ku9HZgixR@4$?EgLCx5=w1(W
z++HcW=#vRl4nu>L*GIvFb71w6BxQ_plMcrMZZbW8A}-CX5SUUxlgRxREZXgyyquvB
zklP?@WA{RnLct8?X*H;i+DnzUO?N#Jg}QQc1#gtVR3dgsxKVlOZ6qNb
zP8-wv7#1NaR?`bns;I>EN}xO&>DGB7=J!+K2hydlX6<5lo!GoAA1av#*P2@zk0qEy
z&0o<(tYi;sO@7C)4=%LQFO7FvH&r%=NF%OHQcD4W%c0YPNAg)fF6+*QxBMzr1<6v`
zwy{WD-Z^$HOWK=zdHZSg;GiCjE2P~VfQp}7YMhR_FPSV_%C>NbJg?-eddpC(s={x=1XUQu=?t65pokk`0L6{E4_uUb{b!kKna%EynrTgBL+fg<6$dMXeR(nd?
zVa>@na0`fC`axy7A^T)S9b7$#~Ix0h{Dok3kVoD@m+u6;2T`JGb^UzHbM5
z8*piEHZxWlmbRQUcH6s;m
z<6)XyxXb&elN$(bZ6|vRFd_fYief|xAeiw~b!qu){Nk`#`obXN5Cu4`b9CS2p1-&Q
zxpa%E{OcC$Hz~+c_rAb3O(F4w-9he$J+Eo#)OdvWQ{R@_zv6BgSA%6rm~aFk&omK=
zzmtzVN)x{&{kKF=Je3fy7!G56cuj!tPdJ;y(;ia|Cr1n0*S}D09U+bZ?_Wrr`M9E&
zO+J6f9M$gs4R{R5NT~ESQy0`2o*{Lhl{!y46Y)Ig-%NOO()`}!5oVGz3P$b#wYt@!
zOG7?R`b!~{h1U|89H#s=tF4@&%(}*yDP8tsiQ72Q>&_IB@}nzU5GMZ0#6WzHZ#Zh|
zMOB&=qunxyn(M*strTOD2O^6Xy@K`unU9we4a@m6MUS75CDvG0vt5|1Gw;t{=R4J`
z%)`I`pQ)~(4YxuI{D}m7hO~do@Mm4*&s6u1Ldoxh_g9(3FQO%+m6K5F4C(fnn|t(x
zKh`r1KALLHbtL1Y?xB~l
z__ph5s+9*0@+X_V-lJ3CzpX%0z&$@N`0R_}wOxikR=^&y+jep=DqpRvOP5`39lV
zaiDiD(%Ehg5rOn8Fxu4CY+*p?liOq)F}nl(dno(5G7`HA`QeGwlfre;`m&aGMy~j6Pl@-p6u76+%k&
zUL{Vi402?U|MdEm+hYCzc$fIY;~~5!LJO4Nqijlq(r;JZjfV+?EwocXDO-9SW^*r%
z=RcFgLIfu$xHZ2|^qEDdNEb6qYY*B?`L1vsSzHP-bz-a~7FnBtl}TsTo^$mSE84yU
zzpk0v2*k4uUrw&iJG+<1{M5mrL;u0o5ViE#4_C%%Kl-@z>SG;xQ#40Ca<*WWldtOW
z)9UKcqakGv3-)f7&%Q#}jke)w-albHJQ&0$2H!+ocOjI51?7*CtLq)#eIp+Kkua+Gs0G|ye#2;hq4_TOZ*>QyjdRZ$ywOq
ztIflLWeWp^uUK(QgBzO#GBkrr>7?)(Kh>x@2kY~}+`MVlbC=48G31?)1LMX|G+VHY
zrd!L#QS%>u`zWUVj#J}f!&!jg&J+cmvz&bn)%Wt)vB1v-B^f)u6JAF=DJH;F$1SGs
zUM0lA8-6R+l8Gua$AAG&9x8E2U>RMTs6MV%ttjam&{9k*rP9|>RbU)dfvA%PRubY-HK)_#{H}jy`qePqP7fI#Pgd2>sms@
zTwIZFUNY1e#6D@UyE}^xWfGdHYmnq@2a$y=SGvKj{-uVbD_Bz~FX4*C<&U7RVYBRr
zP7*>$d58vY1GoM)K9qY|k<`Lrmk+N>;QbYLM)vmqLmeD$e_mM{{k8;Lc#X6NIJyg@
zcJi7TUE6u`xo-gYOcuDW@>)`rSnDf6f-pb6rKfIOX09{4aWpvgNur)d{d^`45OeEc
zW5tfTkp9>7EJI91;WJu0wqWpO>U}2PQNxC|5OS^(g^Sbhmr|6RX1=*p=<&>zy-!*B
zRV)Lsvxu|FL6(%HT`L`IXU;Z+`tB8WLm;SD+(R;4;uL8Q5@|7BD)#B?qV#FlJA>K<
zG9>)CC1zwRLeoTbC^z%v%J!?b{EX#3D$zbu1!}vQ-7?x3BF+m7T
z6IRP=rT&^U%BL}t+zT-f>oCAX%a8SEt8i$vfZjGA|IiaiY?~Up&88xaw#XVh!7#q{
z6?v`U@y5khfIupsn?jT5WaCR>+S`ofw?nv}fcP4&y+r|=RpRAP+R|esfNNH}xd+d8
z-C9giPi*2e-WzVl3BjP8P5urABBZ6bI5Hcw(pm4BkI+0w%EtFY==obmC>P5@tdnE(
zhi`)mioeebARw~AxzE2J?D^0B{%8D`V?L@1e^>DLBM1Kx{5gL9S#ke;0O79S-8SrR
z(LQ*xai=MJSNQKupx>eZKq$s9;r~xd=&qi-oq^w)8nOQ0NBpBta97LSa^-I=c6k3<
zyu7R6ZkqL5!TG~q3VtVDcSY|~+TWs`B!7tBCAfDr+@*}aHRQsb`fBh>TyQ}o?jNrFQ0KlFC0Qd(#xGVm5&;D2Ob*jIJ|LNyd6;R+91OTw$
OzejKwPyVFRfd2#kj9@(g
literal 0
HcmV?d00001
From e9c7d0a65cc478586bf36da2cd4c4b1451b04042 Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Fri, 3 Feb 2023 22:54:46 -0800
Subject: [PATCH 2/4] Case-sensitive Directory Name
Not a problem on my Windows system.
---
tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
index 3c13589746..fdad623409 100644
--- a/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
+++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
@@ -10,7 +10,7 @@ class RowBreakTest extends TestCase
{
public function testReadAndWriteRowBreak(): void
{
- $file = 'tests/data/Reader/Xlsx/issue.3143a.xlsx';
+ $file = 'tests/data/Reader/XLSX/issue.3143a.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
$worksheet = $spreadsheet->getActiveSheet();
From b35318abedd402dbbbd433c1190fd5127ad363fb Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Sun, 5 Feb 2023 19:23:16 -0800
Subject: [PATCH 3/4] Update Documentation and Add Tests
Change is necessitated by probable Excel bug.
---
docs/topics/recipes.md | 9 ++++
src/PhpSpreadsheet/Worksheet/Worksheet.php | 2 +
.../Reader/Xlsx/RowBreakTest.php | 49 ++++++++++++++++++-
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/topics/recipes.md b/docs/topics/recipes.md
index f5563ef96a..691d081284 100644
--- a/docs/topics/recipes.md
+++ b/docs/topics/recipes.md
@@ -479,6 +479,15 @@ row 10.
$spreadsheet->getActiveSheet()->setBreak('A10', \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_ROW);
```
+If your print break is inside a defined print area, it may be necessary to add an extra parameter to specify the max column (and this probably won't hurt if the break is not inside a defined print area):
+
+```php
+$spreadsheet->getActiveSheet()
+ ->setBreak('A10',
+ \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_ROW,
+ \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_ROW_MAX_COLUMN);
+```
+
The following line of code sets a print break on column D:
```php
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
index cb444ac5a0..c4570df87c 100644
--- a/src/PhpSpreadsheet/Worksheet/Worksheet.php
+++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -35,6 +35,8 @@ class Worksheet implements IComparable
public const BREAK_NONE = 0;
public const BREAK_ROW = 1;
public const BREAK_COLUMN = 2;
+ // Maximum column for row break
+ public const BREAK_ROW_MAX_COLUMN = 16383;
// Sheet state
public const SHEETSTATE_VISIBLE = 'visible';
diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
index fdad623409..9bb75f6355 100644
--- a/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
+++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/RowBreakTest.php
@@ -3,6 +3,8 @@
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
use PHPUnit\Framework\TestCase;
@@ -13,12 +15,55 @@ public function testReadAndWriteRowBreak(): void
$file = 'tests/data/Reader/XLSX/issue.3143a.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
- $worksheet = $spreadsheet->getActiveSheet();
+ $sheet = $spreadsheet->getActiveSheet();
$writer = new XlsxWriter($spreadsheet);
$writerWorksheet = new XlsxWriter\Worksheet($writer);
- $data = $writerWorksheet->writeWorksheet($worksheet, []);
+ $data = $writerWorksheet->writeWorksheet($sheet, []);
$expected = '';
self::assertStringContainsString($expected, $data);
$spreadsheet->disconnectWorksheets();
}
+
+ public function testWriteRowBreakInPrintAreaWithMax(): void
+ {
+ // This test specifies max for setBreak and appears correct.
+ $spreadsheet = new Spreadsheet();
+ $sheet = $spreadsheet->getActiveSheet();
+ for ($row = 1; $row < 60; ++$row) {
+ for ($column = 'A'; $column !== 'L'; ++$column) {
+ $cell = $column . $row;
+ $sheet->getCell($cell)->setValue($cell);
+ }
+ }
+ $sheet->getPageSetup()->setPrintArea('B2:J55');
+ $sheet->setBreak('A25', Worksheet::BREAK_ROW, Worksheet::BREAK_ROW_MAX_COLUMN);
+ $writer = new XlsxWriter($spreadsheet);
+ $writerWorksheet = new XlsxWriter\Worksheet($writer);
+ $data = $writerWorksheet->writeWorksheet($sheet, []);
+ $expected = '';
+ self::assertStringContainsString($expected, $data);
+ $spreadsheet->disconnectWorksheets();
+ }
+
+ public function testWriteRowBreakInPrintAreaWithoutMax(): void
+ {
+ // This test does not specify max for setBreak,
+ // and appears incorrect. Probable Excel bug.
+ $spreadsheet = new Spreadsheet();
+ $sheet = $spreadsheet->getActiveSheet();
+ for ($row = 1; $row < 60; ++$row) {
+ for ($column = 'A'; $column !== 'L'; ++$column) {
+ $cell = $column . $row;
+ $sheet->getCell($cell)->setValue($cell);
+ }
+ }
+ $sheet->getPageSetup()->setPrintArea('B2:J55');
+ $sheet->setBreak('A25', Worksheet::BREAK_ROW);
+ $writer = new XlsxWriter($spreadsheet);
+ $writerWorksheet = new XlsxWriter\Worksheet($writer);
+ $data = $writerWorksheet->writeWorksheet($sheet, []);
+ $expected = '';
+ self::assertStringContainsString($expected, $data);
+ $spreadsheet->disconnectWorksheets();
+ }
}
From cc0e93434d97f552136592d42e803a59d7fe82a1 Mon Sep 17 00:00:00 2001
From: oleibman <10341515+oleibman@users.noreply.github.com>
Date: Wed, 8 Feb 2023 20:26:04 -0800
Subject: [PATCH 4/4] Unhappy With Initial Implementation
I kind of shoe-horned it in. Better to create a new PageBreak class, which will make it easier to accomodate any future surprises about page break handling. The only difficulty with the new approach is making sure getBreaks maintains backwards compatibility. New tests will ensure that.
---
src/PhpSpreadsheet/Worksheet/PageBreak.php | 58 +++++++
src/PhpSpreadsheet/Worksheet/Worksheet.php | 56 ++++---
src/PhpSpreadsheet/Writer/Html.php | 2 +-
src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 26 +--
src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 19 ++-
.../Worksheet/PageBreakTest.php | 148 ++++++++++++++++++
6 files changed, 261 insertions(+), 48 deletions(-)
create mode 100644 src/PhpSpreadsheet/Worksheet/PageBreak.php
create mode 100644 tests/PhpSpreadsheetTests/Worksheet/PageBreakTest.php
diff --git a/src/PhpSpreadsheet/Worksheet/PageBreak.php b/src/PhpSpreadsheet/Worksheet/PageBreak.php
new file mode 100644
index 0000000000..743cc58d69
--- /dev/null
+++ b/src/PhpSpreadsheet/Worksheet/PageBreak.php
@@ -0,0 +1,58 @@
+breakType = $breakType;
+ $this->coordinate = $coordinate;
+ $this->maxColOrRow = $maxColOrRow;
+ }
+
+ public function getBreakType(): int
+ {
+ return $this->breakType;
+ }
+
+ public function getCoordinate(): string
+ {
+ return $this->coordinate;
+ }
+
+ public function getMaxColOrRow(): int
+ {
+ return $this->maxColOrRow;
+ }
+
+ public function getColumnInt(): int
+ {
+ return Coordinate::indexesFromString($this->coordinate)[0];
+ }
+
+ public function getRow(): int
+ {
+ return Coordinate::indexesFromString($this->coordinate)[1];
+ }
+
+ public function getColumnString(): string
+ {
+ return Coordinate::indexesFromString($this->coordinate)[2];
+ }
+}
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
index c4570df87c..27ef7735a2 100644
--- a/src/PhpSpreadsheet/Worksheet/Worksheet.php
+++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -190,18 +190,18 @@ class Worksheet implements IComparable
private $conditionalStylesCollection = [];
/**
- * Collection of breaks.
+ * Collection of row breaks.
*
- * @var int[]
+ * @var PageBreak[]
*/
- private $breaks = [];
+ private $rowBreaks = [];
/**
- * Collection of max column values for row breaks.
+ * Collection of column breaks.
*
- * @var int[]
+ * @var PageBreak[]
*/
- private $rowBreaksMax = [];
+ private $columnBreaks = [];
/**
* Collection of merged cell ranges.
@@ -1757,19 +1757,16 @@ public function duplicateConditionalStyle(array $styles, $range = '')
*
* @return $this
*/
- public function setBreak($coordinate, $break, int $rowMax = -1)
+ public function setBreak($coordinate, $break, int $max = -1)
{
$cellAddress = Functions::trimSheetFromCellReference(Validations::validateCellAddress($coordinate));
if ($break === self::BREAK_NONE) {
- if (isset($this->breaks[$cellAddress])) {
- unset($this->breaks[$cellAddress]);
- }
- } else {
- $this->breaks[$cellAddress] = $break;
- if ($break === self::BREAK_ROW) {
- $this->rowBreaksMax[$cellAddress] = $rowMax;
- }
+ unset($this->rowBreaks[$cellAddress], $this->columnBreaks[$cellAddress]);
+ } elseif ($break === self::BREAK_ROW) {
+ $this->rowBreaks[$cellAddress] = new PageBreak($break, $cellAddress, $max);
+ } elseif ($break === self::BREAK_COLUMN) {
+ $this->columnBreaks[$cellAddress] = new PageBreak($break, $cellAddress, $max);
}
return $this;
@@ -1801,12 +1798,35 @@ public function setBreakByColumnAndRow($columnIndex, $row, $break)
*/
public function getBreaks()
{
- return $this->breaks;
+ $breaks = [];
+ foreach ($this->rowBreaks as $break) {
+ $breaks[$break->getCoordinate()] = self::BREAK_ROW;
+ }
+ foreach ($this->columnBreaks as $break) {
+ $breaks[$break->getCoordinate()] = self::BREAK_COLUMN;
+ }
+
+ return $breaks;
}
- public function getRowBreakMax(string $cellAddress): int
+ /**
+ * Get row breaks.
+ *
+ * @return PageBreak[]
+ */
+ public function getRowBreaks()
+ {
+ return $this->rowBreaks;
+ }
+
+ /**
+ * Get row breaks.
+ *
+ * @return PageBreak[]
+ */
+ public function getColumnBreaks()
{
- return $this->rowBreaksMax[$cellAddress] ?? -1;
+ return $this->columnBreaks;
}
/**
diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php
index 309168f25b..c30bb30acf 100644
--- a/src/PhpSpreadsheet/Writer/Html.php
+++ b/src/PhpSpreadsheet/Writer/Html.php
@@ -1193,7 +1193,7 @@ private function generateRowStart(Worksheet $worksheet, $sheetIndex, $row)
{
$html = '';
if (count($worksheet->getBreaks()) > 0) {
- $breaks = $worksheet->getBreaks();
+ $breaks = $worksheet->getRowBreaks();
// check if a break is needed before this row
if (isset($breaks['A' . $row])) {
diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
index ce6a84f206..79ab874c4b 100644
--- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
+++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
@@ -2022,27 +2022,15 @@ private function writeBreaks(): void
$vbreaks = [];
$hbreaks = [];
- foreach ($this->phpSheet->getBreaks() as $cell => $breakType) {
+ foreach ($this->phpSheet->getRowBreaks() as $cell => $break) {
// Fetch coordinates
$coordinates = Coordinate::coordinateFromString($cell);
-
- // Decide what to do by the type of break
- switch ($breakType) {
- case \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_COLUMN:
- // Add to list of vertical breaks
- $vbreaks[] = Coordinate::columnIndexFromString($coordinates[0]) - 1;
-
- break;
- case \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_ROW:
- // Add to list of horizontal breaks
- $hbreaks[] = $coordinates[1];
-
- break;
- case \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::BREAK_NONE:
- default:
- // Nothing to do
- break;
- }
+ $hbreaks[] = $coordinates[1];
+ }
+ foreach ($this->phpSheet->getColumnBreaks() as $cell => $break) {
+ // Fetch coordinates
+ $coordinates = Coordinate::indexesFromString($cell);
+ $vbreaks[] = $coordinates[0] - 1;
}
//horizontal page breaks
diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
index 7ced3ed23f..d9b6e2df60 100644
--- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
+++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php
@@ -1006,12 +1006,11 @@ private function writeBreaks(XMLWriter $objWriter, PhpspreadsheetWorksheet $work
// Get row and column breaks
$aRowBreaks = [];
$aColumnBreaks = [];
- foreach ($worksheet->getBreaks() as $cell => $breakType) {
- if ($breakType == PhpspreadsheetWorksheet::BREAK_ROW) {
- $aRowBreaks[] = $cell;
- } elseif ($breakType == PhpspreadsheetWorksheet::BREAK_COLUMN) {
- $aColumnBreaks[] = $cell;
- }
+ foreach ($worksheet->getRowBreaks() as $cell => $break) {
+ $aRowBreaks[$cell] = $break;
+ }
+ foreach ($worksheet->getColumnBreaks() as $cell => $break) {
+ $aColumnBreaks[$cell] = $break;
}
// rowBreaks
@@ -1020,13 +1019,13 @@ private function writeBreaks(XMLWriter $objWriter, PhpspreadsheetWorksheet $work
$objWriter->writeAttribute('count', (string) count($aRowBreaks));
$objWriter->writeAttribute('manualBreakCount', (string) count($aRowBreaks));
- foreach ($aRowBreaks as $cell) {
+ foreach ($aRowBreaks as $cell => $break) {
$coords = Coordinate::coordinateFromString($cell);
$objWriter->startElement('brk');
$objWriter->writeAttribute('id', $coords[1]);
$objWriter->writeAttribute('man', '1');
- $rowBreakMax = $worksheet->getRowBreakMax($cell);
+ $rowBreakMax = $break->getMaxColOrRow();
if ($rowBreakMax >= 0) {
$objWriter->writeAttribute('max', "$rowBreakMax");
}
@@ -1042,11 +1041,11 @@ private function writeBreaks(XMLWriter $objWriter, PhpspreadsheetWorksheet $work
$objWriter->writeAttribute('count', (string) count($aColumnBreaks));
$objWriter->writeAttribute('manualBreakCount', (string) count($aColumnBreaks));
- foreach ($aColumnBreaks as $cell) {
+ foreach ($aColumnBreaks as $cell => $break) {
$coords = Coordinate::coordinateFromString($cell);
$objWriter->startElement('brk');
- $objWriter->writeAttribute('id', (string) (Coordinate::columnIndexFromString($coords[0]) - 1));
+ $objWriter->writeAttribute('id', (string) ((int) $coords[0] - 1));
$objWriter->writeAttribute('man', '1');
$objWriter->endElement();
}
diff --git a/tests/PhpSpreadsheetTests/Worksheet/PageBreakTest.php b/tests/PhpSpreadsheetTests/Worksheet/PageBreakTest.php
new file mode 100644
index 0000000000..cbeb96d68d
--- /dev/null
+++ b/tests/PhpSpreadsheetTests/Worksheet/PageBreakTest.php
@@ -0,0 +1,148 @@
+getActiveSheet();
+ $sheet->setBreak('A20', Worksheet::BREAK_ROW);
+ $sheet->setBreak('A40', Worksheet::BREAK_ROW);
+ $sheet->setBreak('H1', Worksheet::BREAK_COLUMN);
+ $sheet->setBreak('X1', Worksheet::BREAK_COLUMN);
+ $breaks1 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'A40' => Worksheet::BREAK_ROW,
+ 'H1' => Worksheet::BREAK_COLUMN,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks1
+ );
+ $sheet->setBreak('A40', Worksheet::BREAK_NONE);
+ $sheet->setBreak('H1', Worksheet::BREAK_NONE);
+ $sheet->setBreak('XX1', Worksheet::BREAK_NONE);
+ $breaks2 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks2
+ );
+ $spreadsheet->disconnectWorksheets();
+ }
+
+ public function testBreaksArray(): void
+ {
+ $spreadsheet = new Spreadsheet();
+ $sheet = $spreadsheet->getActiveSheet();
+ $sheet->setBreak([1, 20], Worksheet::BREAK_ROW);
+ $sheet->setBreak([1, 40], Worksheet::BREAK_ROW);
+ $sheet->setBreak([8, 1], Worksheet::BREAK_COLUMN);
+ $sheet->setBreak([24, 1], Worksheet::BREAK_COLUMN);
+ $breaks1 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'A40' => Worksheet::BREAK_ROW,
+ 'H1' => Worksheet::BREAK_COLUMN,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks1
+ );
+ $sheet->setBreak([1, 40], Worksheet::BREAK_NONE);
+ $sheet->setBreak([8, 1], Worksheet::BREAK_NONE);
+ $sheet->setBreak([50, 1], Worksheet::BREAK_NONE);
+ $breaks2 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks2
+ );
+ $spreadsheet->disconnectWorksheets();
+ }
+
+ public function testBreaksCellAddress(): void
+ {
+ $spreadsheet = new Spreadsheet();
+ $sheet = $spreadsheet->getActiveSheet();
+ $sheet->setBreak(new CellAddress('A20'), Worksheet::BREAK_ROW, 16383);
+ $sheet->setBreak(new CellAddress('A40', $sheet), Worksheet::BREAK_ROW);
+ $sheet->setBreak(new CellAddress('H1'), Worksheet::BREAK_COLUMN);
+ $sheet->setBreak(new CellAddress('X1', $sheet), Worksheet::BREAK_COLUMN);
+ $breaks1 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'A40' => Worksheet::BREAK_ROW,
+ 'H1' => Worksheet::BREAK_COLUMN,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks1
+ );
+ $sheet->setBreak(new CellAddress('A40'), Worksheet::BREAK_NONE);
+ $sheet->setBreak(new CellAddress('H1', $sheet), Worksheet::BREAK_NONE);
+ $sheet->setBreak(new CellAddress('XX1'), Worksheet::BREAK_NONE);
+ $breaks2 = $sheet->getBreaks();
+ self::assertSame(
+ [
+ 'A20' => Worksheet::BREAK_ROW,
+ 'X1' => Worksheet::BREAK_COLUMN,
+ ],
+ $breaks2
+ );
+ $spreadsheet->disconnectWorksheets();
+ }
+
+ public function testBreaksOtherMethods(): void
+ {
+ $spreadsheet = new Spreadsheet();
+ $sheet = $spreadsheet->getActiveSheet();
+ $sheet->setBreak('A20', Worksheet::BREAK_ROW, 16383);
+ $sheet->setBreak('A40', Worksheet::BREAK_ROW);
+ $sheet->setBreak('H1', Worksheet::BREAK_COLUMN);
+ $sheet->setBreak('X1', Worksheet::BREAK_COLUMN);
+
+ $rowBreaks = $sheet->getRowBreaks();
+ self::assertCount(2, $rowBreaks);
+ self::assertSame(Worksheet::BREAK_ROW, $rowBreaks['A20']->getBreakType());
+ self::assertSame('A20', $rowBreaks['A20']->getCoordinate());
+ self::assertSame(16383, $rowBreaks['A20']->getMaxColOrRow());
+ self::assertSame(1, $rowBreaks['A20']->getColumnInt());
+ self::assertSame('A', $rowBreaks['A20']->getColumnString());
+ self::assertSame(20, $rowBreaks['A20']->getRow());
+ self::assertSame(Worksheet::BREAK_ROW, $rowBreaks['A20']->getBreakType());
+ self::assertSame('A40', $rowBreaks['A40']->getCoordinate());
+ self::assertSame(-1, $rowBreaks['A40']->getMaxColOrRow());
+ self::assertSame(1, $rowBreaks['A40']->getColumnInt());
+ self::assertSame('A', $rowBreaks['A40']->getColumnString());
+ self::assertSame(40, $rowBreaks['A40']->getRow());
+ self::assertSame(Worksheet::BREAK_ROW, $rowBreaks['A40']->getBreakType());
+
+ $columnBreaks = $sheet->getColumnBreaks();
+ self::assertCount(2, $columnBreaks);
+ self::assertSame(Worksheet::BREAK_COLUMN, $columnBreaks['H1']->getBreakType());
+ self::assertSame('H1', $columnBreaks['H1']->getCoordinate());
+ self::assertSame(8, $columnBreaks['H1']->getColumnInt());
+ self::assertSame('H', $columnBreaks['H1']->getColumnString());
+ self::assertSame(1, $columnBreaks['H1']->getRow());
+ self::assertSame(Worksheet::BREAK_COLUMN, $columnBreaks['H1']->getBreakType());
+ self::assertSame('X1', $columnBreaks['X1']->getCoordinate());
+ self::assertSame(24, $columnBreaks['X1']->getColumnInt());
+ self::assertSame('X', $columnBreaks['X1']->getColumnString());
+ self::assertSame(1, $columnBreaks['X1']->getRow());
+ self::assertSame(Worksheet::BREAK_COLUMN, $columnBreaks['X1']->getBreakType());
+ $spreadsheet->disconnectWorksheets();
+ }
+}