Re: Reduce duplicate code: PrinterJob::writeProlog

2012-03-14 Thread Michael Meeks
Hi Christina,

On Mon, 2012-03-05 at 14:59 +0100, Chr. Rossmanith wrote:
 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.

It sounded at the end of it all, as if this code was utterly unused and
the make check pieces un-necessary (if it was failing), are you happy
with pushing this as-is ? :-)

Thanks !

Michael.

-- 
michael.me...@suse.com  , Pseudo Engineer, itinerant idiot

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: Reduce duplicate code: PrinterJob::writeProlog

2012-03-14 Thread Chr. Rossmanith

Am 14.03.2012 15:42, schrieb Michael Meeks:

Hi Christina,

On Mon, 2012-03-05 at 14:59 +0100, Chr. Rossmanith wrote:

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.

It sounded at the end of it all, as if this code was utterly unused and
the make check pieces un-necessary (if it was failing), are you happy
with pushing this as-is ? :-)
Yes that's fine. Could you push it for me, please? I have some commits 
queued up and don't want them to be pushed all together - and don't 
remember how I've managed to push a single commit.


Christina
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: Reduce duplicate code: PrinterJob::writeProlog

2012-03-05 Thread 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.

-- 
michael.me...@suse.com  , Pseudo Engineer, itinerant idiot

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: Reduce duplicate code: PrinterJob::writeProlog

2012-03-05 Thread Chr. Rossmanith

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.



From ee4bfbaecc7f99d01165be73c237d83dccf2f668 Mon Sep 17 00:00:00 2001
From: Christina Rossmanith chrrossman...@web.de
Date: Mon, 5 Mar 2012 14:57:30 +0100
Subject: [PATCH] Remove SO52 strict compatibility stuff

---
 vcl/generic/print/common_gfx.cxx |3 +-
 vcl/generic/print/genprnpsp.cxx  |   36 ---
 vcl/generic/print/printerjob.cxx |   89 +-
 vcl/generic/print/text_gfx.cxx   |   42 --
 vcl/headless/svpprn.cxx  |   11 -
 vcl/inc/generic/printergfx.hxx   |3 -
 6 files changed, 2 insertions(+), 182 deletions(-)

diff --git a/vcl/generic/print/common_gfx.cxx b/vcl/generic/print/common_gfx.cxx
index 941a5cc..44bf1a7 100644
--- a/vcl/generic/print/common_gfx.cxx
+++ b/vcl/generic/print/common_gfx.cxx
@@ -129,8 +129,7 @@ PrinterGfx::PrinterGfx() :
 maFillColor (0xff,0,0),
 maTextColor (0,0,0),
 maLineColor (0, 0xff, 0),
-mpFontSubstitutes( NULL ),
-mbStrictSO52Compatibility( false )
+mpFontSubstitutes( NULL )
 {
 maVirtualStatus.mfLineWidth = 1.0;
 maVirtualStatus.mnTextHeight = 12;
diff --git a/vcl/generic/print/genprnpsp.cxx b/vcl/generic/print/genprnpsp.cxx
index a456517..64f86b3 100644
--- a/vcl/generic/print/genprnpsp.cxx
+++ b/vcl/generic/print/genprnpsp.cxx
@@ -415,18 +415,6 @@ void SalGenericInstance::configurePspInfoPrinter(PspSalInfoPrinter *pPrinter,
 pJobSetup-maPrinterName= pQueueInfo-maPrinterName;
 pJobSetup-maDriver = aInfo.m_aDriverName;
 copyJobDataToJobSetup( pJobSetup, aInfo );
-
-// set/clear backwards compatibility flag
-bool bStrictSO52Compatibility = false;
-boost::unordered_maprtl::OUString, rtl::OUString, rtl::OUStringHash ::const_iterator compat_it =
-pJobSetup-maValueMap.find( rtl::OUString( RTL_CONSTASCII_USTRINGPARAM( StrictSO52Compatibility ) ) );
-
-if( compat_it != pJobSetup-maValueMap.end() )
-{
-if( compat_it-second.equalsIgnoreAsciiCaseAsciiL(RTL_CONSTASCII_STRINGPARAM(true)) )
-bStrictSO52Compatibility = true;
-}
-pPrinter-m_aPrinterGfx.setStrictSO52Compatibility( bStrictSO52Compatibility );
 }
 }
 
@@ -629,18 +617,6 @@ sal_Bool PspSalInfoPrinter::Setup( SalFrame* pFrame, ImplJobSetup* pJobSetup )
 // should be merged into the independent data
 sal_Bool PspSalInfoPrinter::SetPrinterData( ImplJobSetup* pJobSetup )
 {
-// set/clear backwards compatibility flag
-bool bStrictSO52Compatibility = false;
-boost::unordered_maprtl::OUString, rtl::OUString, rtl::OUStringHash ::const_iterator compat_it =
-pJobSetup-maValueMap.find( rtl::OUString( RTL_CONSTASCII_USTRINGPARAM( StrictSO52Compatibility ) ) );
-
-if( compat_it != pJobSetup-maValueMap.end() )
-{
-if( 

Re: Reduce duplicate code: PrinterJob::writeProlog

2012-03-05 Thread Caolán McNamara
On Mon, 2012-03-05 at 11:41 +, Michael Meeks wrote:
   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 ?

Originally the smoketest just wrote out some .sdX files and reloaded
them, and since write capability was removed from binfilter that wasn't
possible anymore, so I justed lumped in the last .sdX files we were able
to write into qadevOOo/testdocs/ for the purposes of the smoketest
binfilter test, there's no thought gone into their specific contents.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice