Norbert Thiebaud wrote (25 februari 2012 11:26) > ... >why is P78275 removed ? > ... >same question for these 2 > ... >and these 3 etc...
I have have removed some obsolete labels. I even removed one complete brand (Herlitz) after communication with the manufacturer. Herlitz labels are no longer produced and no longer on the market. >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 I understand what you mean. It started with 'just' adding 2 values (i.e. page width and page height). To add these, I had to verify the paper dimensions when the calculated dimensions were not exactly A4, A5, Letter. Whilst verifying I found sevral obsolete 9and unverify-able labels and removed them. The diff file produced by these changes was that large, that I added the lay-out changes, which didn't really make the diff file much bigger. So I focussed on size of the diff file and not on readability/ease of review. >#define RC_LABFMT_BEGIN (RC_ENVELP_BEGIN + 50) >-#define RC_LABFMT_END (RC_ENVELP_BEGIN + 59) >+#define RC_LABFMT_END (RC_ENVELP_BEGIN + 62) > ... >why the shuffling of constants here ? These constants give the ranges for UI-constants (controls, text labels, etc.). I have added two text labels and two input fields to the label dimensions tab in the label wizard dialog. These fout did not fit in the range provided, so I had to enlarge the range, with the consequence that all ranges after LABFMT had to be moved as well. >- SetFillColor(rWinColor); >+ SetFillColor( Color( 0xE0, 0xE0, 0xFF ) ); >Why the magic numbers for the fill color ? This colour is used for the label in the graphic representation of the label with its dimensions. Originally it was an image in shades of gray (background, paper, label) and I wanted the label to come out better. Seeing no inconspicous standard colour, I defined one myself. With hindsight it would probably have been better to make a constant for this colour. It is not good practice to leave such colour definitions in the code. I hope my explanations will help you. I not, please say so. Winfried _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice