Re: [Libreoffice] [REVIEW] Easy patch on l10ntools

2012/1/3 Eike Rathke <erack@redhat.com>:
> Hi julien2412,
>
> On Wednesday, 2011-12-28 14:31:21 -0800, julien2412 wrote:
>
> > Cppcheck report tells this :
> > 1474  duplicateBranch style   Found duplicate branches for if and else
> > for l10ntools/source/export.cxx
> >
> >
> > Even if this seems easy, perhaps it could be something else than just
> > "rText" here.
> >
> > diff --git a/l10ntools/source/export.cxx b/l10ntools/source/export.cxx
> > index bd1fbfb..fbed184 100644
> > --- a/l10ntools/source/export.cxx
> > +++ b/l10ntools/source/export.cxx
> > @@ -1475,7 +1475,7 @@ void Export::InsertListEntry( const ByteString &rText,
> > const ByteString &rLine )
> >              (*pCurEntry)[ SOURCE_LANGUAGE ] = rLine;
> >          }
> >          else
> > -            (*pCurEntry)[ SOURCE_LANGUAGE ] = rLine;
> > +            (*pCurEntry)[ SOURCE_LANGUAGE ] = rText;
> >
> >          pList->NewSourceLanguageListEntry();
> >      }
> >
> > Your opinion ?
>
> Makes sense, especially when seen with context of the block immediately
> above:
>
>    // For paired list use the line to set proper lid
>    if( nList == LIST_PAIRED ){
>        (*pCurEntry)[ m_sListLang ] = rLine;
>    }else
>        (*pCurEntry)[ m_sListLang ] = rText;

On the other hand, if it changes the behaviour that we got used to, it
may break localizations. I'd prefer to have a full code analysis.

Best regards,
Andras