Re: [Evolution-hackers] Patch proposal to e-msg-composer and -attachment-bar

[Jeffrey Stedfast <fejj novell com>]

I'm not sure I like that approach because it just feels dirty that
e-msg-composer would be poking private data in the
EMsgComposerAttachmentBar widget.

I have a couple ideas for solving this a cleaner way:

1. Perhaps EMsgComposerAttachmentBar should be the widget that displays
the label, rather than EMsgComposer itself.

2. Perhaps an e_msg_composer_attachment_bar_get_attachments() function
can be aded to get the list of attachments. However, I'm not sure I like
this mostly because it'd return a GList with type-less data... I don't
like the idea that the composer would have to know that the void* data
are really EMsgComposerAttachment items.

... had another idea but I forgot it in the midst of writing down my
other 2 ideas. oh well, maybe it'll come to me later.

Perhaps Zucchi will have some more ideas too.

Jeff

On Mon, 2005-06-27 at 22:36 +0200, smurfd wrote:
> Hey guys, again, sorry to bother!
> 
> Now i belive i have figured out what was causing my previous error, and
> got a pretty good theory why.
> 
> Seeing how _EMsgComposerAttachmentBarPrivate is only typedef:ed in
> e-msg-composer-attachment-bar.h to EMsgComposerAttachmentBar there's not
> much help to include that .h file. In e-msg-composer-attachment-bar.c
> the struct got its "body" declared. If you dont define the
> _EMsgComposerAttachmentBarPrivate structs "body" in the .c file you plan
> to use a priv. variable in, it wont work, for a good reason.
> 
> The compiler/builder/linker wouldnt know what that type would contain,
> right?.
> 
> In this case it would mean that i would just have to put a "body" to the
> struct in the top of e-msg-composer.c file. like : 
> 
> struct _EMsgComposerAttachmentBarPrivate {
> 	GtkWidget *attach;/* attachment file dialogue, if active */
> 	GList *attachments;
> 	guint num_attachments;
> 	gchar *path;
> };
> 
> then do what my previous get_attachment_size_string() function did, in
> the callback function to the attachment bar.
> 
> Does this sound acceptable/sane to you?!
> This way, only 1 file has been edited.
> 
> Or is there any better way?
> 
> regards 
> /Nicklas
> 
> sön 2005-06-26 klockan 10:04 -0400 skrev Jeffrey Stedfast:
> > please use diff -up next time
> > 
> > On Sun, 2005-06-26 at 16:40 +0200, smurfd wrote:
> > > Hey!
> > > 
> > > Using the cvs diff function, was indeed easy.
> > > 
> > > Im hoping the sewage has atleast turned into a small fork or
> > > something ;)
> > > 
> > > I choosed to keep the get_attachment_size_string() function, due to the
> > > fact that when trying to access a bar->priv->attachments in
> > > e-msg-composer.c (even though, e-msg-composer-attachment-bar.h is
> > > included in it) i did get some error complaining that i couldnt do that
> > > way... dont remember exactly what the errormessage was...
> > > 
> > > I did a "patch -p0 < file.patch" in my evolution/ source dir, all went
> > > fine. Updated from anon cvs a couple of hours ago, so it Should apply
> > > cleanily.
> > > 
> > > regards 
> > > /Nicklas
> > > 
> > > ---- patch ----
> > > ? composer/.e-msg-composer.c.swp
> > > ? composer/mail-composer.error
> > > Index: composer/e-msg-composer-attachment-bar.c
> > > ===================================================================
> > > RCS
> > > file: /cvs/gnome/evolution/composer/e-msg-composer-attachment-bar.c,v
> > > retrieving revision 1.99
> > > diff -r1.99 e-msg-composer-attachment-bar.c
> > > 403a404,413
> > > > gchar* get_attachment_size_string (EMsgComposerAttachmentBar *bar) {
> > 
> > please don't use this style. This should be:
> > 
> > char *
> > get_attachment_size_string (EMsgComposerAttachmentBar *bar)
> > {
> > 
> > however, if this is going to be a public function, it needs to be
> > properly namespaced. However, I don't see why it has to be public - in
> > fact I'd say with certainty that it should not be.
> > 
> > > >       GList *p;
> > > >       gulong size = 0;
> > > >
> > > >       for (p = bar->priv->attachments; p != NULL; p = p->next)
> > > >               size = size + E_MSG_COMPOSER_ATTACHMENT(p->data)->size;
> > > >
> > > >       return size_to_string(size);
> > > > }
> > > >
> > > Index: composer/e-msg-composer-attachment-bar.h
> > > ===================================================================
> > > RCS
> > > file: /cvs/gnome/evolution/composer/e-msg-composer-attachment-bar.h,v
> > > retrieving revision 1.18
> > > diff -r1.18 e-msg-composer-attachment-bar.h
> > > 74c74
> > > <
> > > ---
> > > > gchar* get_attachment_size_string (EMsgComposerAttachmentBar *bar);
> > > Index: composer/e-msg-composer.c
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/composer/e-msg-composer.c,v
> > > retrieving revision 1.512
> > > diff -r1.512 e-msg-composer.c
> > > 122a123
> > > > #include "e-msg-composer-attachment.h"
> > > 2303d2303
> > > <
> > > 2305a2306,2307
> > > >       GList *p;
> > > >
> > > 2308,2311c2310,2315
> > > <                       ngettext ("<b>%d</b> Attachment", "<b>%d</b>
> > > Attachments", attachment_num),
> > > <                       attachment_num);
> > > <               gtk_label_set_markup (GTK_LABEL
> > > (composer->attachment_expander_num),
> > > <                                     num_text);
> > > ---
> > > >                       ngettext ("<b>%d</b> Attachment, %s",
> > > >                       "<b>%d</b> Attachments, %s total",
> > > >
> > > attachment_num),attachment_num,get_attachment_size_string(bar));
> > > >               gtk_label_set_markup (GTK_LABEL(
> > > >                       composer->attachment_expander_num),num_text);
> > 
> > why not put the code for get_attachment_size_string() right here? Why is
> > it even in another source file when it's never used anywhere else but
> > here?
> > 
> > Jeff
> > 
> > _______________________________________________
> > evolution-hackers maillist  -  evolution-hackers lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-hackers
> 
> _______________________________________________
> evolution-hackers maillist  -  evolution-hackers lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-hackers
> 
-- 
Jeffrey Stedfast
Evolution Hacker - Novell, Inc.
fejj ximian com  - www.novell.com