[Kde-pim] Review Request: making kmail network-aware

[Andras Mantia]

Martin Bednár wrote:

> Sure, but the point was that when I was using ==Solid::Connected,
> isOffline returned true for all Solid::Status except Connected, which made
> me see the "askToGoOnline" window a lot. Now We check for !=Unconnected ||
> !=Disconnecting. Solid apparently being smart, uses the Unknown status
> when it doesn't know, which I think is why it works on your setup (and
> mine too). I admit I hadn't realized that when I wrote the second patch.
> The thing here is now that the window will be shown when KMail tries
> checking for mail eg the "Check Mail" button when the network is
> Unconnected || Disconnecting (checked for in isOffline), which is unwanted
> because kmail can't do anything with the global network state.
> 

I admit, I'm confused. :)

You added the check fo ==Solid::Connected originally, which is wrong, as 
Unknown might also mean that it is connected. That's what I'm corrected and 
what you do in line 940 of the patch:
-  if ( Solid::Networking::status()==Solid::Networking::Connected ) {
+  if (  ( Solid::Networking::status() != Solid::Networking::Unconnected ) 
|| ( Solid::Networking::status() != Solid::Networking::Disconnecting ) ) {


This part is fine.

The other part is not fine:
-  if ( kmkernel->isOffline() ) {
+  if ( GlobalSettings::self()->networkState() == 
GlobalSettings::EnumNetworkState::Offline ) {
     s_askingToGoOnline = true;
     int rc =
     KMessageBox::questionYesNo( KMKernel::self()->mainWin(),


if you look at the kmkernel->isOffline implementation it looks like this:
bool KMKernel::isOffline()
{
  if ( ( GlobalSettings::self()->networkState() == 
GlobalSettings::EnumNetworkState::Offline ) ||
       ( Solid::Networking::status() == Solid::Networking::Unconnected ) || 
( Solid::Networking::status() == Solid::Networking::Disconnecting ) )
    return true;
  else
    return false;
}


The first part of the if statment is the same as the change you added. So 
when GlobalSettings::self()->networkState() == 
GlobalSettings::EnumNetworkState::Offline is true, isOffline() returns true, 
WITHOUT checking the solid status (the rest of the if statement is not 
evaluated at all). That one is checked only if GlobalSettings::self()-
>networkState() returns something else, but Offline.

That's why I said, your change is not needed, as it doesn't really change 
the behavior of the application.

I will commit the first part of the patch. 
If you consider to help KDE/KDEPIM more in the future, you can easily apply 
for a developer account.

Andras
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/