Reduce duplicate code: PrinterJob::writeProlog

Chr. Rossmanith ChrRossmanith at gmx.de
Mon Mar 5 05:59:56 PST 2012


Hi,

as suggested on IRC I've removed the whole StrictSO52Compatibility 
stuff. It builds successfully - make test is on its way. "git grep" 
reported some test documents matching as well, so I expect some tests to 
fail.

Christina


Am 05.03.2012 12:41, schrieb Michael Meeks:
> On Fri, 2012-03-02 at 21:03 +0100, Chr. Rossmanith wrote:
>> static const sal_Char pProlog[] and static const sal_Char
>> pSO52CompatProlog[] share about 90% of their content (2 out of 84 lines
>> differ).
> 	Wow - that is really horrible :-)
>
>>   I could split the prolog in three parts: part 1 and 3 identical
>> for both variables and part 2 varies with
>> m_pGraphics->getStrictSO52Compatibility()
> 	Sounds good to me; I'm trying to think of a way to make that
> elegant :-) the prolog for the prolog seems fun :-)
>
> 	There is no performance concern here, so making the code look pretty is
> the only interesting thing I suppose. Your idea of splitting into
> several named variables makes good sense though.
>
>> This is where the prologs are used:
>>       WritePS (pFile, m_pGraphics&&
>> m_pGraphics->getStrictSO52Compatibility() ? pSO52CompatProlog : pProlog);
>>
>> Any arguments against that approach? It would need 3 instead of 1 call
>> to WritePS...
> 	That's no problem at all :-)
>
> 	Thanks for unwinding that cut/paste rat's nest :-)
>
> 	Having said all this, I'm not 100% certain that we really need:
>
> 	m_pGraphics->getStrictSO52Compatibility()
>
> 	is 'strict Star Office 5.2 Compatibility' in postscript generation a
> truly useful feature ? :-) I suspect it could only be at all useful for
> unit testing / backwards compatibility testing - but ...
>
> 	Surely we could update those tests to a new postscript header ?
>
> 	git grep -5 StrictSO52Compatibility
>
> 	I'd be inclined to deliberately break that method, then run 'make
> check' to see if it busts anything.
>
> 	Caolan - I notice the option seems to be mentioned in:
>
> 	qadevOOo/testdocs/ttt.sda
>
> 	Which you touched last :-) is this a feature we need to keep ?
>
> 	All the best,
>
> 		Michael.
>



More information about the LibreOffice mailing list