Re: [Libreoffice] [LibreOffice][PATCH] Fixes Compiler Warning

Jan Holesovsky schreef op vr 15-10-2010 om 22:18 [+0200]:
> Hi Kenneth,
>
> On 2010-10-15 at 21:10 +0200, Kenneth Venken wrote:
>
> > Fixes a compiler warning: "suggest parentheses around && within || "
> > I also improved the readability of the code a bit.
>
> The change you did silents the warning by doing what the compiler would
> do; but is that really what was the intention there?
>
> (aComplexLanguageLB.GetSavedValue() !=
> aComplexLanguageLB.GetSelectEntryPos())
>
> is evaluated regardless the aCTLSupportCB.IsChecked() state - is that
> intentional, can you please check?
Thank you for making me think about the code. I now see why
'warning-free code' is listed under 'slightly more interesting
hacks'. ;)

Correct expression has to be: 
aCTLSupportCB.IsChecked() && 
          (aCTLSupportCB.GetSavedValue() != STATE_CHECK || 
           aComplexLanguageLB.GetSavedValue() !=
aComplexLanguageLB.GetSelectEntryPos())

because sequence checking should only be turned on for certain languages
like Thai, Laotian, Khmer or Vietnamese and only when CTL is enabled.
So, if CTL support is checked, we only need to check for sequence
checking if: previously the CTL support was unchecked or the CTL was
checked but the language has changed.

This compiler warning uncovered a small bug because the old version
would run the code inside the if-statement even when it didn't have to.
Luckily it would always run the code when it had to as well.

*patch attached*

> Thank you,
> Kendy

diff --git a/cui/source/options/optgdlg.cxx b/cui/source/options/optgdlg.cxx
index 41f470e..068d325 100644
--- a/cui/source/options/optgdlg.cxx
+++ b/cui/source/options/optgdlg.cxx
@@ -1512,8 +1512,9 @@ BOOL OfaLanguagesTabPage::FillItemSet( SfxItemSet& rSet )
     pLangConfig->aLanguageOptions.BlockBroadcasts( TRUE );
     pLangConfig->aLinguConfig.BlockBroadcasts( TRUE );
 
-    if( (aCTLSupportCB.IsChecked() && (aCTLSupportCB.GetSavedValue() != STATE_CHECK)) ||
-            (aComplexLanguageLB.GetSavedValue() != aComplexLanguageLB.GetSelectEntryPos()))
+    if( aCTLSupportCB.IsChecked() && 
+          (aCTLSupportCB.GetSavedValue() != STATE_CHECK || 
+           aComplexLanguageLB.GetSavedValue() != aComplexLanguageLB.GetSelectEntryPos()) )
     {
         //sequence checking has to be switched on depending on the selected CTL language
         LanguageType eCTLLang = aComplexLanguageLB.GetSelectLanguage();