Re: [Libreoffice] [REVIEW] bugfixes which break the string freeze for 3.4.3

Hi Peter,

On Wed, 2011-07-27 at 13:21 +0200, Péter Rabi wrote:
> The bug (fdo#32895), which we are talking about affects 'dlgass' (the
> welcoming wizard of impress) and dialogue 'doctempl' (Impress -> File ->
> Templates -> Save).

        Okay ! :-)

> While 'doctempl' uses the template handling module of sfx2,
> unfortunately, 'dlgass' uses TemplateScanner (inside sd/source/ui/{inc,
> gui}) which seems to be a re-implementation of a subset of the services
> granted by the template handling module in sfx2.

        Riight - so this 're-implementation' stuff needs to be consolidated
wherever we see it: that is useful work saving us compile time,
run-time, and download size. The trick of course is to try to work out
why there are two copies ;-) and then add conditionals to the sfx2 one
to behave as expected, I suppose. Can you do that, at least in master.

> So that's why I copy/pasted the mentioned code snippet.

        Ah - but copy/paste is -the- pre-eminent sin of programming.

> Sorry, but I don't really get what do you mean by 'tad baroque'.

        Let me explain:

> + if( rString == ResId::toString( (const ResId)SfxResId( (sal_uInt16)(nSourceResIds + i) ) ) )
> > + {
> > + return ResId::toString( (const ResId)SfxResId( (sal_uInt16)(nDestResIds + i) ) );
> > + }
> > + }
> >     do ? it looks a tad baroque to me.

        Well - its function is not that clear to me :-) can we really not
compare rtl::OUString with String ? the casting frenzy is a bit painful
and un-necessary too, the compiler knows an SfxResId is a ResId
(surely?) ;-) too - so this:

    for( int i = 0; i < nCount; ++i )
    {
        if( rString == ResId::toString( SfxResId( nSourceResIds + i) ) )
            return ResId::toString( SfxResId( nDestResIds + i ) );
    }

        should be just as good, and easier on the eye too ;-) It is also safer
- since adding redundant un-checked casts has a habit of coming back to
bite you later :-)

> >     Can we get a cleaner reading patch, that re-uses that in impress ? :-)
>
> I think the best would be to make Impress/ui stuff use the sfx2 template
> handling and eliminate TemplateScanner. Would you like me to do that?

        That'd be nice - of course, prolly worth cleaning up the master commit
as per above, and knocking up a cleaner / simpler patch to review vs.
3.4.x :-)

        ATB,

                Michael.

-- 
 michael.meeks@novell.com  <><, Pseudo Engineer, itinerant idiot