Re: [evolution-patches] Patch for bug#310479 (Exchange-component)

[shakti <shprasad novell com>]

Yeah, I missed that 'folder' check part. Thanks for pointing it out.
I corrected those small issues and committed the patch.

Here in this patch, though it is not the right way to fix bug #310493, 
atleast it won't crash. I will create one more patch to show/create  the 
custom and separator widgets (A FIXME is there anyway :) ) for bug #310493.

Thnaks for the review comments. The patch is committed.

Thanks,
Shakti


Sarfraaz Ahmed wrote:

> Looks good. Few more comments below. Please correct them and commit.
>
> -- Sarfraaz
>  
>
> > > >       if (role == E2K_PERMISSIONS_ROLE_CUSTOM) {
> > > > -               gtk_widget_show (dialog->priv->separator);
> > > > -               gtk_widget_show (dialog->priv->custom);
> > > > +               /* FIXME: To show Custom */
> > > >               role = E2K_PERMISSIONS_ROLE_NUM_ROLES + 1;
> > > >
> > > > What is this for ?
> > > >   
> > > >
> > > >        
> > This is just to show a label named 'Custom' (I am not showing that
> > label 
> > now, it will show empty). This part fixes bug #310493.
> >
> > I dont think this is the right fix for 310493. We need to be creating
> > the custom and separator widgets as a fix for this. You would want to
> > have a separate patch for it may be.
> >
> >
> > +       source = e_source_selector_peek_primary_selection
> > (E_SOURCE_SELECTOR (target->selector));
> > +       uri = (gchar *) e_source_get_uri (source);
> > +
> > +       if (! g_strrstr (uri, "exchange://") && !folder)        {
> > +               return ;
> >
> > Oops ... we cant check for folder here :). It should be removed from
> > this place. Its there below anyway.
> >
> > +       if (! g_strrstr (uri, "exchange://") && !folder)        {
> > +               return ;
> >
> > Similar.
> >
> > +       }
> > +
> > +       account = exchange_operations_get_exchange_account ();
> > +
> > +       if (!account)
> > +               return;
> > +
> > +
> > +       folder = exchange_account_get_folder (account, uri);
> > +
> > +       if (!folder) {
> > +               return;
> > +       }
> > +
> > +       selected_exchange_folder_uri = uri;
> > +
> > +       /* for translation*/
> > +       if (first) {
> >
> > Hmm, this should (!first) i suppose ? And in that other place as well.
> >
> >        if (! g_strrstr (t->uri, "exchange://") && !folder)
> >                return ;
> >
> > Similar ...
> >
> >        selected_exchange_folder_uri = t->uri;
> >        /* for translation*/
> >        if (first) {
> >
> > And this too now i suppose ??
> >
> >
> >    
>
> -- Sarfraaz
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches
>