Re: [Evolution-hackers] SoC project status

[Not Zed <notzed ximian com>]

Hi David,

You should really do a standalone build, you shouldn't need the
evolution source to build it anymore, well for evolution 2.4 - which is
what you should be targetting.

Looking at the source i'd suggest some minor things: non-public
functions should be static, and any public ones should have obviously
unique names.  'format_part', is not likely to be unique - symbols need
to be globally unqiue to the application otherwise funny things can
happen.  As a standalone the style is up to you, but for consideration
for inclusion in evolution, you'd have to follow the style in HACKING.

Your algorithm for finding conversations is probably not adequate.
Subject alone isn't much use.  Thankfully we already have a re-usable
object which can calculate conversation threads, and is the one used for
the threaded message view.  See camel/camel-folder-thread.h

    // If it's a multipart....
    else if(CAMEL_IS_MULTIPART(content))
    {
        // Get the first part...
        CamelMimePart* part =
camel_multipart_get_part(CAMEL_MULTIPART(content), 0);
        CamelDataWrapper* part_content =
camel_medium_get_content_object(CAMEL_MEDIUM(part));

        // See if there's usable text...
        if (CAMEL_IS_STREAM(part_content->stream))
        {

Stuff like this above is really no good.  If you walk the message
structure, to grab text parts out, you need to honour all of the
relevent formatting rules.  e.g. multipart/mixed is handled differently
to multipart/alternative.  You need to do proper decoding of the parts
if they've been encoded, and handle character set conversions etc.
Infact peeking the 'stream' directly is a no-no, it is essentially a
private structure.  You also have to check things like
camel_multipart_get_part() don't return NULL, etc.

For your 'xxx weeks/days/months' ago code, you probably want to have a
more intelligent month calculator, otherwise it just looks funny.

Your strip_re code is particularly inefficient and complicated.  Using a
char * the code requires no memory management, no recursion, and at most
some very simple pointer arithmetic.  camel-folder-thread and other
places inside evolution have simple and reliable strip_re functions.

       // Now we simply swap away the real message and trick it into
displaying our conversation.
        camel_medium_set_content_object(CAMEL_MEDIUM(target->message),
CAMEL_DATA_WRAPPER(conversation));

Oh dear, definitely do not do this.  The message in the target must be
considered read-only.  You definitely cannot use the 'message.reading'
event for this.  Events are only for notification, not for any
processing.

 Michael

On Tue, 2005-08-23 at 00:45 -0600, David Morrison wrote:
> Well, I've got something together that sortof resembles a working plugin
> -- or at least, I think it works.  However, I'd really like to get some
> other people to try it out and poke a bunch of holes in it, because it's
> not worth anything if it only works for me.
> 
> I have a module called evo-conversation in the Gnome CVS repository, in
> there is instructions to compile and install the plugin.  I also have
> created a tarball with the libraries and an install script, which I'm
> currently hosting at http://www.cs.hmc.edu/~dmorrison/cview.tar.gz
> 
> You can also see my blog at http://blogs.gnome.org/dmorr
> 
> Anyways, let me know what you guys think!
> 
> Thanks,
> David Morrison
> _______________________________________________
> evolution-hackers maillist  -  evolution-hackers lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-hackers
-- 
adfa(evolution-2.4:20087): gtkhtml-WARNING **: cannot find icon:
'stock_insert-url' in gnome