Re: [evolution-patches] Proxy Login Patches
- From: Not Zed <notzed ximian com>
- To: Shreyas <sshreyas novell com>
- Cc: Evolution Patches <evolution-patches lists ximian com>, psankar novell com
- Subject: Re: [evolution-patches] Proxy Login Patches
- Date: Thu, 07 Jul 2005 00:50:55 +0800
On Wed, 2005-07-06 at 15:58 +0530, Shreyas wrote:
> Index: e-util/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/ChangeLog,v
> retrieving revision 1.544
> diff -u -p -w -r1.544 ChangeLog
> --- e-util/ChangeLog 18 Jun 2005 15:59:56 -0000 1.544
> +++ e-util/ChangeLog 5 Jul 2005 09:31:12 -0000
> @@ -1,3 +1,9 @@
> +2005-07-05 Shreyas Srinivasan
> +
> + * e-util/e-account.[ch]: Add structures to handle proxies.
> + * e-util/e-account-list.[ch]: Add functions to remove proxy
> + accounts and account's proxies.
> +
> 2005-06-18 Tor Lillqvist <tml novell com>
>
> * Makefile.am (WIN32_BOOTSTRAP_LIBS): Use bootstrap library
> for
> Index: e-util/e-account.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/e-account.h,v
> retrieving revision 1.9
> diff -u -p -w -r1.9 e-account.h
> --- e-util/e-account.h 8 Apr 2005 04:34:04 -0000 1.9
> +++ e-util/e-account.h 5 Jul 2005 09:31:29 -0000
> @@ -69,6 +69,9 @@ typedef enum _e_account_item_t {
> E_ACCOUNT_SMIME_ENCRYPT_TO_SELF,
> E_ACCOUNT_SMIME_ENCRYPT_DEFAULT,
>
> + E_ACCOUNT_PROXY_PARENT_URL,
Dont store the parent url. You can get that, and SHOULD get that from
the account item which you can get from the parent uid.
> + E_ACCOUNT_PROXY_PARENT_UID,
> +
> E_ACCOUNT_ITEM_LAST
> } e_account_item_t;
>
> @@ -98,6 +101,11 @@ typedef struct _EAccountService {
> gboolean save_passwd;
> } EAccountService;
>
> +typedef struct _EAccountProxyInfo {
> + char *parent_url;
> + char *parent_uid;
> +} EAccountProxyInfo;
Because you dont need all this, just store the parent_uid as a simple
pointer on the main EAccount.
> typedef struct _EAccount {
> GObject parent_object;
>
> @@ -109,6 +117,7 @@ typedef struct _EAccount {
> EAccountIdentity *id;
> EAccountService *source;
> EAccountService *transport;
> + EAccountProxyInfo *proxy_account;
i.e. not this.
> char *drafts_folder_uri, *sent_folder_uri;
>
> @@ -130,6 +139,7 @@ typedef struct _EAccount {
> gboolean smime_sign_default;
> gboolean smime_encrypt_to_self;
> gboolean smime_encrypt_default;
> + GList *proxy_children;
You dont need to store this at all, it can be determined by iterating
over all accounts and checking if the parent uid matches the current
one. It saves having to try to keep a bunch of different values up to
date. The parent uid is the only thing which can't change, so only
store that.
> } EAccount;
>
> typedef struct {
> Index: e-util/e-account.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/e-account.c,v
> retrieving revision 1.15
> diff -u -p -w -r1.15 e-account.c
> --- e-util/e-account.c 8 Apr 2005 04:34:04 -0000 1.15
> +++ e-util/e-account.c 5 Jul 2005 09:32:58 -0000
> @@ -107,6 +107,10 @@ e_account_init (EAccount *account)
> account->id = g_new0 (EAccountIdentity, 1);
> account->source = g_new0 (EAccountService, 1);
> account->transport = g_new0 (EAccountService, 1);
> + account->proxy_account = g_new0 (EAccountProxyInfo, 1);
> +
> + account->proxy_account->parent_url = NULL;
> + account->proxy_account->parent_uid = NULL;
>
> account->source->auto_check = FALSE;
> account->source->auto_check_time = 10;
> @@ -139,6 +143,17 @@ service_destroy (EAccountService *servic
> }
>
> static void
> +proxy_destroy (EAccountProxyInfo *proxy_info)
> +{
> + if (proxy_info->parent_url)
> + g_free (proxy_info->parent_url);
> + if (proxy_info->parent_uid)
> + g_free (proxy_info->parent_uid);
> +
> + g_free (proxy_info);
> +}
> +
> +static void
> e_account_finalize (GObject *object)
> {
> EAccount *account = E_ACCOUNT (object);
> @@ -149,7 +164,11 @@ e_account_finalize (GObject *object)
> identity_destroy (account->id);
> service_destroy (account->source);
> service_destroy (account->transport);
> + proxy_destroy (account->proxy_account);
>
> + g_list_foreach (account->proxy_children, (GFunc) g_free,
> NULL);
> + g_list_free (account->proxy_children);
> + account->proxy_children = NULL;
> g_free (account->drafts_folder_uri);
> g_free (account->sent_folder_uri);
>
> @@ -486,6 +505,17 @@ e_account_set_from_xml (EAccount *accoun
> }
> }
> }
> + } else if (!strcmp (node->name, "proxy")) {
> + if (node->children) {
> + for (cur = node->children; cur; cur =
> cur->next) {
> + if (!strcmp (cur->name,
> "parent-source-url")) {
> + changed |=
> xml_set_content (cur, &account->proxy_account->parent_url);
> + } else if (!strcmp (cur->name,
> "parent-uid")) {
> + changed |=
> xml_set_content (cur, &account->proxy_account->parent_uid);
> + break;
> + }
> + }
> + }
> }
Do you need to save these settings? Dont they just need to be around at
runtime? Do you need this to communicate the values around? Aren't
they stored on the server anyway?
If they dont need to be saved, then you dont need to clear the proxies
on startup, and you just dont save them to gconf when they are created,
or need to handle loading them. But if they are needed for
comminication then i guess there's not much that can be done about
it ...
Once the EAccountList is loaded for the mailer (&used by the composer),
each object will remain in memory, it shouldn't be overwritten even when
a change to the object is made - it is only other copies of eaccountlist
that matter.
> }
>
> @@ -564,6 +594,7 @@ e_account_import (EAccount *dest, EAccou
> g_free (dest->smime_encrypt_key);
> dest->smime_encrypt_key = g_strdup (src->smime_encrypt_key);
>
> + dest->proxy_children = g_list_copy(src->proxy_children);
> g_signal_emit(dest, signals[CHANGED], 0, -1);
> }
>
> @@ -652,6 +683,12 @@ e_account_to_xml (EAccount *account)
> if (account->smime_encrypt_key)
> xmlNewTextChild (node, NULL, "encrypt-key-id",
> account->smime_encrypt_key);
>
> + if (account->proxy_account &&
> account->proxy_account->parent_url) {
> + node = xmlNewChild (root, NULL, "proxy", NULL);
> + xmlNewTextChild (node, NULL, "parent-source-url",
> account->proxy_account->parent_url);
> + xmlNewTextChild (node, NULL, "parent-uid",
> account->proxy_account->parent_uid);
> + }
> +
> xmlDocDumpMemory (doc, &xmlbuf, &n);
> xmlFreeDoc (doc);
>
> @@ -778,6 +815,9 @@ static struct _account_info {
> { /* E_ACCOUNT_SMIME_SIGN_DEFAULT */ 0, TYPE_BOOL,
> G_STRUCT_OFFSET(EAccount, smime_sign_default) },
> { /* E_ACCOUNT_SMIME_ENCRYPT_TO_SELF */ 0, TYPE_BOOL,
> G_STRUCT_OFFSET(EAccount, smime_encrypt_to_self) },
> { /* E_ACCOUNT_SMIME_ENCRYPT_DEFAULT */ 0, TYPE_BOOL,
> G_STRUCT_OFFSET(EAccount, smime_encrypt_default) },
> +
> + { /* E_ACCOUNT_PROXY_PARENT_URL, */ 0, TYPE_STRING|
> TYPE_STRUCT, G_STRUCT_OFFSET(EAccount, proxy_account),
> G_STRUCT_OFFSET(EAccountProxyInfo, parent_url) },
> + { /* E_ACCOUNT_PROXY_PARENT_UID, */ 0, TYPE_STRING|
> TYPE_STRUCT, G_STRUCT_OFFSET(EAccount, proxy_account),
> G_STRUCT_OFFSET(EAccountProxyInfo, parent_uid) },
> };
>
> static GHashTable *ea_option_table;
> Index: e-util/e-account-list.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/e-account-list.h,v
> retrieving revision 1.4
> diff -u -p -w -r1.4 e-account-list.h
> --- e-util/e-account-list.h 31 Mar 2004 10:08:38 -0000 1.4
> +++ e-util/e-account-list.h 5 Jul 2005 09:33:13 -0000
> @@ -71,5 +71,7 @@ void e_account_list_remove
> const EAccount *e_account_list_get_default(EAccountList *);
> void e_account_list_set_default(EAccountList *, EAccount
> *);
> const EAccount *e_account_list_find (EAccountList *,
> e_account_find_t type, const char *key);
> +void e_account_list_prune_proxies (EAccountList *);
> +void e_account_list_update_parent (EAccountList *, EAccount *);
>
> #endif /* __E_ACCOUNT_LIST__ */
> Index: e-util/e-account-list.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/e-account-list.c,v
> retrieving revision 1.10
> diff -u -p -w -r1.10 e-account-list.c
> --- e-util/e-account-list.c 21 Dec 2004 15:50:38 -0000 1.10
> +++ e-util/e-account-list.c 5 Jul 2005 09:33:32 -0000
> @@ -293,6 +293,56 @@ e_account_list_save (EAccountList *accou
> gconf_client_suggest_sync (account_list->priv->gconf, NULL);
> }
>
> +void
> +e_account_list_prune_proxies (EAccountList *account_list)
> +{
> + EAccount *account;
> + EIterator *iter;
> +
> + for (iter = e_list_get_iterator (E_LIST (account_list));
> + e_iterator_is_valid (iter);
> + e_iterator_next (iter)) {
> + account = (EAccount *)e_iterator_get (iter);
> + if (account->proxy_account->parent_url) {
> + e_account_list_remove (account_list, account);
> + e_account_list_save (account_list);
You don't want to save the list every time it changes. Only do it once
after you're finished, if you made any changes.
> + }
> + }
> + g_object_unref (iter);
> +}
> +
> +void
> +e_account_list_update_parent (EAccountList *accounts, EAccount
> *account)
> +{
> + char *parent = NULL;
> + EAccount *parent_account;
> +
> + parent = e_account_get_string (account,
> E_ACCOUNT_PROXY_PARENT_URL);
> + parent_account = e_account_list_find (accounts,
> E_ACCOUNT_FIND_UID, account->proxy_account->parent_uid);
> + if (parent_account) {
> + parent_account->proxy_children = g_list_remove
> (parent_account->proxy_children, account->uid);
> + e_account_list_change (accounts, parent_account);
> + }
This is one of the reasons you dont want to keep an account list or
parent url on the account. If you get rid of those, the need for this
function above evaporates.
> +
> +void
> +e_account_list_remove_account_proxies (EAccountList *accounts,
> EAccount *account)
> +{
> + int i,n;
> + EAccount *child_account;
> + char *uid;
> +
> + n = g_list_length (account->proxy_children);
> + for (i=0; i<n; i++) {
> + uid = g_list_nth_data(account->proxy_children, i);
Ugh, please dont iterate through a list like this, it is excrutiating to
even look at. Iterate it properly, using g_list_next. You've got a
nice O(n*n) algorithm for an inherently O(n) problem.
Anyway, since you're going to remove the proxy_children list anyway,
just iterate over the accounts and remove any that have a parent_uid
which matches the account->uid.
> + child_account = e_account_list_find (accounts,
> E_ACCOUNT_FIND_UID, uid);
> + e_account_list_remove (accounts, child_account);
> + e_account_list_save (accounts);
> + account->proxy_children = g_list_remove
> (account->proxy_children, uid);
> + }
> + g_list_free (account->proxy_children);
> + e_account_list_save (accounts);
> +}
> /**
> * e_account_list_add:
> * @accounts:
> Index: mail/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
> retrieving revision 1.3649
> diff -u -p -w -r1.3649 ChangeLog
> --- mail/ChangeLog 1 Jul 2005 03:29:22 -0000 1.3649
> +++ mail/ChangeLog 5 Jul 2005 10:33:10 -0000
> @@ -1,3 +1,13 @@
> +2005-07-05 Shreyas Srinivasan <sshreyas novell com>
> +
> + * mail-config.[ch]: Add new functions to remove proxy accounts
> and
> + proxies created by the account.
> + * mail-compnent.c: Remove proxy accounts at startup
> + * mail.error.xml: Add warnings for proxy disable and removal
> of a
> + proxy when its parents are removed.
> + * em-account-prefs.c: Add checks not to display proxies and
> + handling of parent removal/ disabling.
> +
> 2005-06-24 Matt Brown <matt mattb net nz>
>
> * em-inline-filter.c: implement extraction of inline
> signed/encrypted pgp
> Index: mail/mail-config.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/mail-config.h,v
> retrieving revision 1.119
> diff -u -p -w -r1.119 mail-config.h
> --- mail/mail-config.h 19 Oct 2004 06:35:44 -0000 1.119
> +++ mail/mail-config.h 5 Jul 2005 10:33:42 -0000
> @@ -133,6 +133,9 @@ struct _EAccountList *mail_config_get_ac
> void mail_config_add_account (struct _EAccount *account);
> void mail_config_remove_account (struct _EAccount *account);
> void mail_config_set_default_account (struct _EAccount *account);
> +void mail_config_update_parent (struct _EAccount *account);
> +void mail_config_remove_account_proxies (struct _EAccount *account);
> +void mail_config_prune_proxies (void);
>
> struct _EAccountIdentity *mail_config_get_default_identity (void);
> struct _EAccountService *mail_config_get_default_transport (void);
> Index: mail/mail-config.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/mail-config.c,v
> retrieving revision 1.317
> diff -u -p -w -r1.317 mail-config.c
> --- mail/mail-config.c 23 Jun 2005 09:11:06 -0000 1.317
> +++ mail/mail-config.c 5 Jul 2005 10:33:53 -0000
> @@ -671,6 +671,25 @@ mail_config_get_account_by_transport_url
> return NULL;
> }
>
> +
> +void
> +mail_config_update_parent (EAccount *account)
> +{
> + e_account_list_update_parent (config->accounts, account);
> +}
Wont be needed if you remove the data duplication in EAccount.
> +void
> +mail_config_remove_account_proxies (EAccount *account)
> +{
> + e_account_list_remove_account_proxies (config->accounts,
> account);
> +}
> +
> +void
> +mail_config_prune_proxies (void)
> +{
> + e_account_list_prune_proxies (config->accounts);
> +}
> +
> EAccountList *
> mail_config_get_accounts (void)
> {
> Index: mail/mail-component.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/mail-component.c,v
> retrieving revision 1.120
> diff -u -p -w -r1.120 mail-component.c
> --- mail/mail-component.c 23 Jun 2005 09:11:06 -0000 1.120
> +++ mail/mail-component.c 5 Jul 2005 10:34:12 -0000
> @@ -359,6 +359,7 @@ mc_startup(MailComponent *mc)
> started = 1;
>
> mc_setup_local_store(mc);
> + mail_config_prune_proxies ();
> load_accounts(mc, mail_config_get_accounts());
> vfolder_load_storage();
> }
> Index: mail/mail.error.xml
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/mail.error.xml,v
> retrieving revision 1.3
> diff -u -p -w -r1.3 mail.error.xml
> --- mail/mail.error.xml 7 Jun 2005 05:05:43 -0000 1.3
> +++ mail/mail.error.xml 5 Jul 2005 10:34:29 -0000
> @@ -249,6 +249,25 @@ The message is stored in the Outbox fold
> <button stock="gtk-no" _label="Don't delete"
> response="GTK_RESPONSE_NO"/>
> </error>
>
> +<error id="ask-delete-account-with-proxies" type="question"
> default="GTK_RESPONSE_NO" modal="true">
> + <_title>Delete account?</_title>
> + <_primary>Are you sure you want to delete this account and all its
> proxies?</_primary>
> + <_secondary xml:space="preserve">If you proceed, the account
> information and
> +all proxy information will be deleted permanently.</_secondary>
> + <button stock="gtk-delete" response="GTK_RESPONSE_YES"/>
> + <button stock="gtk-no" _label="Don't delete"
> response="GTK_RESPONSE_NO"/>
> + </error>
> +
> +<error id="ask-delete-proxy-accounts" type="question"
> default="GTK_RESPONSE_NO" modal="true">
> + <_title>Disable account and log out all proxies?</_title>
> + <_primary>Are you sure you want to disable?</_primary>
> + <_secondary xml:space="preserve">If you proceed, the account will
> be disabled
> +and all its proxies will be logged out.</_secondary>
> + <button stock="gtk-ok" response="GTK_RESPONSE_YES"/>
> + <button stock="gtk-cancel" response="GTK_RESPONSE_NO"/>
> + </error>
> +
> +
dont add 2 spaces here.
> <error id="no-save-signature" type="error">
> <_primary>Could not save signature file.</_primary>
> <_secondary xml:space="preserve">Because
> "{0}".</_secondary>
> Index: mail/em-account-prefs.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-account-prefs.c,v
> retrieving revision 1.19
> diff -u -p -w -r1.19 em-account-prefs.c
> --- mail/em-account-prefs.c 16 May 2005 06:15:37 -0000 1.19
> +++ mail/em-account-prefs.c 5 Jul 2005 10:34:50 -0000
> @@ -192,7 +192,7 @@ account_edit_clicked (GtkButton *button,
> if (gtk_tree_selection_get_selected (selection,
> &model, &iter))
> gtk_tree_model_get (model, &iter, 3, &account,
> -1);
>
> - if (account) {
> + if (account && !account->proxy_account->parent_url
> && !g_list_length(account->proxy_children)) {
> EMAccountEditor *emae;
>
> /** @HookPoint-EMConfig: Mail Account Editor
> @@ -227,6 +227,7 @@ account_delete_clicked (GtkButton *butto
> GtkTreeModel *model;
> GtkTreeIter iter;
> int ans;
> + gboolean has_spawned_proxies = FALSE;
No need to write a book to explain the variable name. The teaching of
computer programming must've changed since I was at uni; we were told
explictly to make variable names readable, but also short and to the
point. has_proxies would be more than enough here.
> selection = gtk_tree_view_get_selection (prefs->table);
> if (gtk_tree_selection_get_selected (selection, &model,
> &iter))
> @@ -236,7 +237,11 @@ account_delete_clicked (GtkButton *butto
> if (account == NULL || prefs->editor != NULL)
> return;
>
> + if (g_list_length(account->proxy_children))
you're going to have to add a helper to find out if it has proxies,
hwich searches all accounts for a proxy entry on that account.
> + ans = e_error_run(PREFS_WINDOW(prefs),
> "mail:ask-delete-account-with-proxies",NULL);
> + else
> ans = e_error_run(PREFS_WINDOW(prefs),
> "mail:ask-delete-account", NULL);
> +
something like this might be neater:
e_error_run(prefs, has_proxies?"error1":"error2", NULL);
> if (ans == GTK_RESPONSE_YES) {
> int len;
>
> @@ -244,6 +249,12 @@ account_delete_clicked (GtkButton *butto
> if (account->enabled && account->source &&
> account->source->url)
> mail_component_remove_store_by_uri
> (mail_component_peek (), account->source->url);
>
> + /* remove all the proxies you created*/
> + if (g_list_length (account->proxy_children)) {
> + has_spawned_proxies = TRUE;
> + mail_config_remove_account_proxies(account);
> + }
> +
> /* remove it from the config file */
> mail_config_remove_account (account);
> accounts = mail_config_get_accounts ();
> @@ -252,6 +263,8 @@ account_delete_clicked (GtkButton *butto
>
> gtk_list_store_remove ((GtkListStore *) model, &iter);
>
> + if (has_spawned_proxies)
> + mail_accounts_load (prefs);
Hmm,I dont follow why this is needed - aren't proxy account entries
never actually shown in the account list anyway? So it doesn't have
anything to remove here ...?
> len = e_list_length ((EList *) accounts);
> if (len > 0) {
> gtk_tree_selection_select_iter (selection,
> &iter);
> @@ -344,17 +357,30 @@ account_able_toggled (GtkCellRendererTog
>
> if (gtk_tree_model_get_iter (model, &iter, path)) {
> gtk_tree_model_get (model, &iter, 3, &account, -1);
> +
> + if (g_list_length (account->proxy_children)) {
> + char *ans;
> + ans = e_error_run(PREFS_WINDOW(prefs),
> "mail:ask-delete-proxy-accounts",NULL);
errr, e_error_run returns an int, not a char *. Didn't you check
compiler warnings????
> + if (ans == GTK_RESPONSE_YES) {
> account->enabled = !account->enabled;
> + e_account_list_change(mail_config_get_accounts(), account);
> gtk_list_store_set ((GtkListStore *) model, &iter, 0,
> account->enabled, -1);
> -
> if (gtk_tree_selection_iter_is_selected (selection,
> &iter))
> gtk_button_set_label (prefs->mail_able,
> account->enabled ? _("Disable") : _("Enable"));
Umm, it appears as though you've added 2 levels of indent here, but
lines like the one above (and others) haven't been moved to match it?
> -
> - /* let the rest of the application know it changed */
> + mail_config_remove_account_proxies
> (account);
> + gtk_widget_set_sensitive (GTK_WIDGET
> (prefs->mail_edit), 1);
Why are you changing the edit sensitivity here? Can't you edit accounts
with proxies? How do you add more proxies? Aren't the proxy accounts
never listed in the first place?
> + account_able_changed (account);
> + }
> + } else {
> + account->enabled = !account->enabled;
> e_account_list_change(mail_config_get_accounts(),
> account);
> + gtk_list_store_set ((GtkListStore *) model,
> &iter, 0, account->enabled, -1);
> + if (gtk_tree_selection_iter_is_selected
> (selection, &iter))
> + gtk_button_set_label
> (prefs->mail_able, account->enabled ? _("Disable") : _("Enable"));
> + /* let the rest of the application know it
> changed */
> account_able_changed(account);
> }
> -
> + }
Again, extra level of indent, no update to the content.
The code above looks a bit confusing - like you've copied a bunch of
code you didn't really need to. Can't you just add a special case
within that block to remove the proxies, rather than adding awhole new
copy of the logic block.
> gtk_tree_path_free (path);
> }
>
> @@ -371,13 +397,17 @@ account_cursor_change (GtkTreeSelection
> EAccount *account = NULL;
> GtkTreeModel *model;
> GtkTreeIter iter;
> + char *url = NULL;
> int state;
>
> +/* This function needs to get the account variable rather than parse
> for stupid proxy = yes*/
> +
indent the comment within functions please, add FIXME if it its a fixme.
> state =
> gconf_client_key_is_writable(mail_config_get_gconf_client(),
> "/apps/evolution/mail/accounts", NULL);
> if (state) {
> state = gtk_tree_selection_get_selected (selection,
> &model, &iter);
> if (state) {
> gtk_tree_model_get (model, &iter, 3, &account,
> -1);
> + url = e_account_get_string (account,
> E_ACCOUNT_SOURCE_URL);
> if (account->source && account->enabled)
> gtk_button_set_label
> (prefs->mail_able, _("Disable"));
> else
> @@ -390,7 +420,12 @@ account_cursor_change (GtkTreeSelection
> gtk_widget_set_sensitive (GTK_WIDGET (prefs), FALSE);
> }
>
> + if( url != NULL ) {
> + if( g_list_length(account->proxy_children))
> + gtk_widget_set_sensitive (GTK_WIDGET
> (prefs->mail_edit), 0);
> + else
> gtk_widget_set_sensitive (GTK_WIDGET (prefs->mail_edit),
> state);
> + }
Umm, this seems to not set the edit button sensitive state if you dont
have a source url - which is a perfectly valid case (no receiving source
set). This logic could be done a lot simpler and not miss a case.
e.g. set_sensitive(mail_edit, state && !account_is_proxy(account))
> gtk_widget_set_sensitive (GTK_WIDGET (prefs->mail_delete),
> state);
> gtk_widget_set_sensitive (GTK_WIDGET (prefs->mail_default),
> state);
> gtk_widget_set_sensitive (GTK_WIDGET (prefs->mail_able),
> state);
> @@ -420,6 +455,7 @@ mail_accounts_load (EMAccountPrefs *pref
>
> account = (EAccount *) e_iterator_get (node);
>
> + if (!account->proxy_account->parent_url) {
> url = account->source && account->source->url ?
> camel_url_new (account->source->url, NULL) : NULL;
>
> gtk_list_store_append (model, &iter);
> @@ -443,6 +479,7 @@ mail_accounts_load (EMAccountPrefs *pref
> camel_url_free (url);
>
> row++;
> + }
Umm, again, more indenting without fixing up the indent of the code
within it. If you don't indent properly it makes the logic much harder
to understand when you look at it.
> e_iterator_next (node);
> }
> Index: composer/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/composer/ChangeLog,v
> retrieving revision 1.700
> diff -u -p -w -r1.700 ChangeLog
> --- composer/ChangeLog 3 Jun 2005 09:52:02 -0000 1.700
> +++ composer/ChangeLog 5 Jul 2005 11:37:48 -0000
> @@ -1,3 +1,8 @@
> +2005-07-05 Shreyas Srinivasan <sshreyas novell com>
> +
> + * e-msg-composer-hdrs.c: Take into account store
> + permissions (Read Only). These are proxy specific as of now.
> +
> 2005-06-03 Srinivasa Ragavan <sragavan novell com>
>
> * e-msg-composer.c (e_msg_composer_attach): Fixed to show the
> Index: composer/e-msg-composer-hdrs.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/composer/e-msg-composer-hdrs.c,v
> retrieving revision 1.137
> diff -u -p -w -r1.137 e-msg-composer-hdrs.c
> --- composer/e-msg-composer-hdrs.c 17 Jun 2005 15:20:28
> -0000 1.137
> +++ composer/e-msg-composer-hdrs.c 5 Jul 2005 13:06:55 -0000
> @@ -56,6 +56,9 @@
> #include "e-util/e-error.h"
>
> #include <camel/camel.h>
> +#include <camel/camel-store.h>
> +#include <camel/camel-session.h>
> +#include <e-util/e-proxy.h>
Ugh no no ... more on this later.
> #include "e-msg-composer-hdrs.h"
> #include "mail/mail-config.h"
> /*#include "mail/em-folder-selection-button.h"*/
> @@ -65,6 +68,7 @@
> /*#include "mail/mail-component.h"*/
> struct _MailComponent *mail_component_peek(void);
> extern struct _EMFolderTreeModel
> *mail_component_peek_tree_model(struct _MailComponent *);
> +CamelSession *session;
This should be extern, or maybe include mail-session.h (mail-component.h
isn't included because of some bonobo related header issues).
> #include "mail/em-folder-tree.h"
>
> @@ -228,6 +232,8 @@ account_added_cb (EAccountList *accounts
> omenu = e_msg_composer_hdrs_get_from_omenu (hdrs);
> menu = gtk_option_menu_get_menu (GTK_OPTION_MENU (omenu));
>
> + if (account_can_write_mails (account))
> + {
c'mon, get the style right.
And if you add a block, you gotta indent it!
account_can_write_mails is a bit bulky, what about account_can_send(),
or even account_check_perms(can_send)
> label = g_strdup_printf ("%s <%s>", account->id->name,
> account->id->address);
> item = gtk_menu_item_new_with_label (label);
> gtk_widget_show (item);
> @@ -245,6 +251,7 @@ account_added_cb (EAccountList *accounts
> toplevel = gtk_widget_get_toplevel ((GtkWidget *) hdrs);
> gtk_widget_set_sensitive (toplevel, TRUE);
> }
> +}
>
> static void
> account_changed_cb (EAccountList *accounts, EAccount *account,
> EMsgComposerHdrs *hdrs)
> @@ -316,6 +323,26 @@ account_removed_cb (EAccountList *accoun
> }
> }
>
> +gboolean
> +account_can_write_mails (EAccount *account)
> +{
> + static CamelStore *store;
> + CamelException ex;
> +
> + if (!account->proxy_account->parent_url)
> + return TRUE;
> +
> + if (!(store = (CamelStore *) camel_session_get_service
> (session, account->source->url, CAMEL_PROVIDER_STORE, &ex))) {
> + camel_exception_clear (&ex);
> + return FALSE;
> + }
> +
> + if (store->permissions & PROXY_MAIL_WRITE)
> + return TRUE;
You can't have a value on a camel object, which is determined by a flag
defined elsewhere. It must be a CAMEL_STORE_* flag, which must be
defined in camel-store.h. A little more on this later.
Since you wont have the parent source url anymore, you'll have to lookup
the parent account - but its only an extra line of code.
Also, try to use the accessors, e_account_get_string(), etc rather than
using account->blah.
> + return FALSE;
> +}
> +
> static GtkWidget *
> create_from_optionmenu (EMsgComposerHdrs *hdrs)
> {
> @@ -341,7 +368,7 @@ create_from_optionmenu (EMsgComposerHdrs
> while (e_iterator_is_valid (iter)) {
> account = (EAccount *) e_iterator_get (iter);
>
> - if (account->id->address)
> + if (account->id->address && account_can_write_mails
> (account))
> g_ptr_array_add (addresses,
> account->id->address);
>
> e_iterator_next (iter);
> @@ -360,7 +387,7 @@ create_from_optionmenu (EMsgComposerHdrs
> continue;
> }
>
> - if (account->id->address && *account->id->address) {
> + if (account->id->address && *account->id->address &&
> account_can_write_mails (account)) {
> /* If the account has a unique email address,
> just
> * show that. Otherwise include the account
> name.
> */
> @@ -1104,6 +1131,8 @@ e_msg_composer_hdrs_set_from_account (EM
> item = l->data;
>
> account = g_object_get_data ((GObject *) item,
> "account");
> + if (account_can_write_mails (account))
> + {
> if (account_name) {
Umm, apart from terrible indenting, what is this logic? Isn't it just a
&& you want? Dont nest two if if's that dont nest other logic but
another if.
> if (account->name && !strcmp (account_name,
> account->name)) {
> /* set the correct optionlist item */
> @@ -1121,7 +1150,7 @@ e_msg_composer_hdrs_set_from_account (EM
>
> return;
> }
> -
> + }
> l = l->next;
> i++;
> }
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution-data-server/camel/ChangeLog,v
> retrieving revision 1.2451
> diff -u -p -w -r1.2451 ChangeLog
> --- ChangeLog 1 Jul 2005 03:30:35 -0000 1.2451
> +++ ChangeLog 5 Jul 2005 14:14:48 -0000
> @@ -1,3 +1,8 @@
> +2005-07-05 Shreyas Srinivasan <sshreyas novell com>
> +
> + * camel-store.h: Add a new #define to identify proxy stores
> + and a new data member permissions to hold store specific
> permissions.
> +
> 2005-06-24 Matt Brown <matt mattb net nz>
>
> * camel-gpg-context.c: Extend verify and decrypt functions to
> support
> Index: camel-store.h
> ===================================================================
> RCS file: /cvs/gnome/evolution-data-server/camel/camel-store.h,v
> retrieving revision 1.75
> diff -u -p -w -r1.75 camel-store.h
> --- camel-store.h 9 May 2005 20:13:13 -0000 1.75
> +++ camel-store.h 5 Jul 2005 14:15:13 -0000
> @@ -112,6 +112,7 @@ typedef struct _CamelRenameInfo {
> #define CAMEL_STORE_VTRASH (1 << 1)
> #define CAMEL_STORE_FILTER_INBOX (1 << 2)
> #define CAMEL_STORE_VJUNK (1 << 3)
> +#define CAMEL_STORE_PROXY (1 << 4)
>
> struct _CamelStore {
> CamelService parent_object;
> @@ -120,6 +121,7 @@ struct _CamelStore {
> CamelObjectBag *folders;
>
> int flags;
> + gint16 permissions;
Again, just remember to define the permission flags here. The e-proxy.h
stuff shouldn't be included either, that seems to be very gw-backend
specific. They can be used to remap the flags in the gw-backend if they
dont match, but if they're the same they wont need to be.
(e-proxy.h should probably remain as part of the e-gw-connection stuff
rather than a separate file, the camel interface should stand alone here
- other backends also have permission possibilities).
> };
>
> /* open mode for folder */
> Index: camel-store.c
> ===================================================================
> RCS file: /cvs/gnome/evolution-data-server/camel/camel-store.c,v
> retrieving revision 1.163
> diff -u -p -w -r1.163 camel-store.c
> --- camel-store.c 9 Jun 2005 07:00:23 -0000 1.163
> +++ camel-store.c 5 Jul 2005 14:15:40 -0000
> @@ -138,6 +138,7 @@ camel_store_init (void *o)
>
> /* set vtrash and vjunk on by default */
> store->flags = CAMEL_STORE_VTRASH | CAMEL_STORE_VJUNK;
> + store->permissions = 0;
Just FYI, you dont need to initialise things to zero (it doesn't hurt
though).
> store->priv = g_malloc0 (sizeof (*store->priv));
> store->priv->folder_lock = e_mutex_new (E_MUTEX_REC);
>
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches
>
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
