Re: [PATCH]bug 44516 improved label and business card document creation

ok so I re-dit the diff to get something humanly reviewable, I applied
on 3.5.1-RC1

git apply original.patch
git diff -w > ../ip/simplified_bug_44516.diff
cat ../ip/simplified_bug_44516.diff | grep -v "^-$" | grep -C2
"^[+\-]" | sed -e "s/-    \( *\)<value>/-\1<value>/" | less

                 </prop>
                 <prop oor:name="Measure">
-                <value>S;10500;14850;10500;14850;0;0;2;2</value>
-                </prop>
-            </node>
-            <node oor:name="L86" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>P78275 Address</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>S;8500;4200;8500;4200;2000;2250;2;</value>
+                <value>S;10500;14850;10500;14850;0;0;2;2;21000;29700</value>
                 </prop>
             </node>
--

why is P78275 removed ?

-                </prop>
-            </node>
-            <node oor:name="L273" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4110 Shipping</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;14605;7620;14605;7461;953;0;1;1</value>
+                <value>C;12700;7620;12700;7461;953;0;1;1;14606;7620</value>
                 </prop>
             </node>
--
                 </prop>
                 <prop oor:name="Measure">
-                <value>C;8890;1270;8890;1111;953;0;1;1</value>
-                </prop>
-            </node>
-            <node oor:name="L275" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4255 File Folder</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;8890;1270;8890;1111;1270;0;1;1</value>
-                </prop>
-            </node>
-            <node oor:name="L276" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4266 File Folder-Assorted</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;8890;1270;8890;1111;1270;0;1;1</value>
+                <value>C;8890;1270;8890;1111;953;0;1;1;10796;1270</value>
                 </prop>
             </node>
--

same question for these 2

-                <value>C;8890;6350;8890;6191;953;0;1;1</value>
-                </prop>
-            </node>
-            <node oor:name="L279" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4241 3-1/2&quot; Diskette</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;6985;7620;6985;6985;2223;0;1;1</value>
-                </prop>
-            </node>
-            <node oor:name="L280" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4166 Index Card</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;12700;7620;12700;7620;1270;0;1;1</value>
-                </prop>
-            </node>
-            <node oor:name="L281" oor:op="replace" oor:finalized="true">
-                <prop oor:name="Name">
-                <value>4167 Post Card</value>
-                </prop>
-                <prop oor:name="Measure">
-                <value>C;15240;8890;15240;8890;1270;0;1;1</value>
+                <value>C;8890;6350;8890;6191;953;0;1;1;10796;6350</value>
                 </prop>
             </node>

and these 3 etc...

so... If these changes are intentional I would urge you to:

 + do the cosmetic white-space/indentation fixes to label.xcu in a
separate patch that contain only these (i.e diff -u -w -> empty diff)
 + do the needed removal addition of labels in a separate patch,
preferably with some explanations in the commit message as to why
 + do the wholesale addition of the 2 new attributes (as I understand
adding these attributes does not _require_ the underlying code change
right ?
 + then a patch with he code change

That would render it possible for someone to really review the changes


diff --git a/sw/inc/envelp.hrc b/sw/inc/envelp.hrc
index b77e92e..1aa09e0 100644
--- a/sw/inc/envelp.hrc
+++ b/sw/inc/envelp.hrc
@@ -63,22 +63,21 @@
 #define RC_LABEL_END     (RC_ENVELP_BEGIN + 49)

 #define RC_LABFMT_BEGIN  (RC_ENVELP_BEGIN + 50)
-#define RC_LABFMT_END    (RC_ENVELP_BEGIN + 59)
+#define RC_LABFMT_END    (RC_ENVELP_BEGIN + 62)

-#define RC_LABPRT_BEGIN  (RC_ENVELP_BEGIN + 60)
-#define RC_LABPRT_END    (RC_ENVELP_BEGIN + 69)
+#define RC_LABPRT_BEGIN  (RC_ENVELP_BEGIN + 63)
+#define RC_LABPRT_END    (RC_ENVELP_BEGIN + 72)

 // Strings --------------------------------------------------------------------

-#define STR_DATABASE_NOT_OPENED  (RC_ENVLOP_BEGIN + 60)
-#define STR_NO_DRIVERS           (RC_ENVLOP_BEGIN + 61)
-#define DLG_MERGE_CREATE         (RC_ENVLOP_BEGIN + 62)
-#define DLG_MERGE_FIELD_CONNECTIONS (RC_ENVLOP_BEGIN + 63)
-
-#define STR_BTN_NEW_DOC          (RC_ENVLOP_BEGIN + 64)
-#define STR_BTN_NEWDOC           (RC_ENVLOP_BEGIN + 65)
-#define STR_SENDER_TOKENS        (RC_ENVLOP_BEGIN + 66)
+#define STR_DATABASE_NOT_OPENED  (RC_ENVLOP_BEGIN + 63)
+#define STR_NO_DRIVERS           (RC_ENVLOP_BEGIN + 64)
+#define DLG_MERGE_CREATE         (RC_ENVLOP_BEGIN + 65)
+#define DLG_MERGE_FIELD_CONNECTIONS (RC_ENVLOP_BEGIN + 66)

+#define STR_BTN_NEW_DOC          (RC_ENVLOP_BEGIN + 67)
+#define STR_BTN_NEWDOC           (RC_ENVLOP_BEGIN + 68)
+#define STR_SENDER_TOKENS        (RC_ENVLOP_BEGIN + 69)

why the shuffling of constants here ?

@@ -170,7 +173,7 @@ void SwLabPreview::Paint(const Rectangle &)

     // Labels
     SetClipRegion (Rectangle(Point(lX0, lY0), Size(lOutlineW, lOutlineH)));
-    SetFillColor(rWinColor);
+    SetFillColor( Color( 0xE0, 0xE0, 0xFF ) );
     for (sal_uInt16 nRow = 0; nRow < Min((sal_uInt16) 2, (sal_uInt16)
aItem.nRows); nRow++)
         for (sal_uInt16 nCol = 0; nCol < Min((sal_uInt16) 2,
(sal_uInt16) aItem.nCols); nCol++)
             DrawRect(Rectangle(

Why the magic numbers for the fill color ?