Re: [evolution-patches] Exchange: Patch for bug #311324

[Sarfraaz Ahmed <asarfraaz novell com>]

Looks good. A few styling/naming/error-check issues. Please correct them
and commit.

Thanks
-- Sarfraaz

@@ -1,3 +1,12 @@
+2005-07-29  Shakti Sen <shprasad novell com>
+
+       * exchange-folder.c (org_gnome_exchange_folder_ab_unsubscribe,
+       org_gnome_exchange_folder_unsubscribe, 
+       org_gnome_exchange_folder_subscription): Added offline/online
support

We usually list the functions in separate braces like .. (func1)(func2)


Index: exchange-config-listener.c
===================================================================


+gboolean
+get_offline_status (ExchangeConfigListener *excl)

The function name should have the proper namespace. It should be called
exchange_config_listener_get_offline_status. 

+{
+       ExchangeConfigListenerPrivate *priv = excl->priv;

You should check for a NULL excl here and return the status. To support
error conditions and not just default to some offline/online state, you
could probably pass the online/offline state in the argument, and have
the function return value report any errors.

+       GConfValue *value;
+       gboolean offline = FALSE;
+       value = gconf_client_get (priv->gconf,
+                                       "/apps/evolution/shell/start_offline", NULL);
+       if (value)
+               offline = gconf_value_get_bool (value);

Add a new line here.

+       return offline;


+       if (mode) {
+               printf ("Unsubscribe to Other User's Folder is not
allowed in Offline mode\n");

We could probably use a g_warning for this now. And file a bug for this
FIXME so that we dont forget it.

+               /* FIXME:
+                  I think throwing an error dialog is not allowed
+                  because of UI freeze.
+                  e_error_run (NULL, ERROR_DOMAIN ":folder-offline-
error", NULL);
+               */
+               return;
+     


+       mode = exchange_is_offline ();
+       if (mode) {
+               printf ("Unsubscribe to Other User's Folder is not
allowed in Offline mode\n");

Similar ...

+               /* FIXME:
+                  I think throwing an error dialog is not allowed 
+                  because of UI freeze.
+                  e_error_run (NULL, ERROR_DOMAIN ":folder-offline-
error", NULL);
+               */
+          


+       mode = exchange_is_offline ();
+       if (mode) {
+               printf ("Subscribe to Other User's Folder is not allowed
in Offline mode\n");

Similar ....

+               /* FIXME:
+                  I think throwing an error dialog is not allowed i
+                  because of UI freeze.
+                  e_error_run (NULL, ERROR_DOMAIN ":folder-offline-
error", NULL);

Index: exchange-operations.c
===================================================================

+gboolean
+exchange_is_offline () {

Add a newline here.

+       return get_offline_status (exchange_global_config_listener);

delimit)
Index: exchange-operations.h
===================================================================
RCS file: /cvs/gnome/evolution/plugins/exchange-operations/exchange-
operations.h,v
retrieving revision 1.5
diff -u -p -r1.5 exchange-operations.h
--- exchange-operations.h       25 Jul 2005 06:12:03 -0000      1.5
+++ exchange-operations.h       1 Aug 2005 05:12:41 -0000
@@ -40,6 +40,8 @@ extern ExchangeConfigListener *exchange_
 int e_plugin_lib_enable (EPluginLib *eplib, int enable);
 
 ExchangeAccount *exchange_operations_get_exchange_account (void);
+gboolean exchange_is_offline (void);
+gboolean get_offline_status (ExchangeConfigListener *excl);

This declaration should go to exchange-config-listener.h