Re: [evolution-patches] Patch to allow a contact to be opened in the editor from command-line.

[Sushma Rai <rsushma novell com>]

Apart from these... a few comments..

about the coding style in addressbook_view_edit_contact(),
It's good if you return on failure and proceed otherwise..

for example
book = e_book_new ();
if  (!book) {
    // log error and return
}
if  (!e_book_open()) {
    // log error and return
}
etc..

You need not mark warning strings for translation.
book and contact needs to be unrefed.
Since you need both source uid and contact uid, it's better
you return on either NULL source UID or contact UID.

Also there are some compiler warnings...
addressbook_view_edit_contact() not declared in the header file,
uid_view, uid are not used.

Thanks,
Sushma.


On Mon, 2005-07-11 at 15:42 +0530, Veerapuram Varadhan wrote:
> On Mon, 2005-07-11 at 15:32 +0800, Not Zed wrote:
> > You shouldn't do anything AFTER the parent dispose method is invoked.
> > 
> I will correct it.  Thanks.
> 
> > Why is it done as query strings, rather than uri parameters like the
> > mailer?  Why can't you specify the source uid as part of the uri
> > specification (i.e. host/usrname) rather than as a query parameter?
> > 
> I just followed the existing URI handling in calendars.  Well,
> source-uid and contact-uids' can be passed as parameters.  However, I am
> not sure how to pass them as part of uri specification (like
> host/username etc). 
> 
> I used them as "query" strings as they sounded like "querying" for a
> contact and opening it.  Let me know the right way to follow, I am sure
> I will correct it.
> 
> > The query decoding is too simple too, it needs to do proper
> > url-decoding
> > of the parameters, if you're going to go that route - which should be
> > avoided anyway (simply because you need to do so much more work than
> > te
> > url parameters already do for you).
> > 
> Yes, I agree, but, the "EUri" already decodes them so, we might not need
> another decoding after that.
> 
> Let me know if I am wrong.
> 
> Thanks,
> 
> V. Varadhan
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches