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

[smurfd <smurfd gmail com>]

Please, maillist administrators. Dissregard my message i sent when i
handt changed the mailaddy to use on the list. the topic then was
something like "patch proposal to e-m-c and e-m-c-a-b (continues)" ..
dissregard from that ..
and hopefully this mail will fall into the other thread of mails with
the same topic. or else im sorry, because my mailserver has died for a
unknown time (or atleast till  early september)

NotZed wrote : 
> The original is better than the last one or this one.
> 
> The problem is when discussing code there's always a lot of ways to do
> the same thing.  Maybe it is an experience thing, and it isn't always
> good - I often see so many ways to solve even simple problems that I'm
> blinded from the more straightforward ones.  So in email or irc what
> comes out is often a rush of partially thought-out possibilities, rarely
> the ideal path, and sometimes not even a reasonable path.

> So I apologise if the various discussions just get you confused, they're
> often thought experiments which aren't meant to go further, rather than
> actual solutions.

Its totaly cool, i guess everyone has different ways of interpeting
speach/chatting/writing/etc and that one has to interpet things
different from different people. Say with some i might have to sort
away lots of things beeing said, and read between the lines, but where
with some one might not have to...

Also i guess it differs from the different kind of topics one discuss.
Its probably true, that speaking solutions of code might sortof be
hard. Seeing how one problem can be solved in various different
sollutions, probably one solution per person.

That is the beauty of the development bussness, aint it? :)

> > something like this perhaps then, i had to re-use the size_to_string()
> > function aswell, since it was a private one too.
> 
> Eek no, definitely not.  Having duplicated structures around is
> definitely a recipe for disaster.  Its also full of c99 stuff like
> in-line variable declarations which are right out, and it still leaks
> the output of size_to_string (at least).
> 
> Ugh, who on earth wrote all that 1e3L mess, hmm, messy.  Whats so
> unreadable about "1024".  *shakes head*  But I digress ...

jup, its much easier to understand if it would have said "size <
1024" instead of how it is now.

i'd probably do it something like :

gchar *
e_msg_composer_attachment_size_to_string (gulong size) {
        gchar *size_string = NULL;
        gdouble displayed_size;

        if (size < 1024) 
            size_string = NULL;
        else {
                if (size < 1024*1024) {
                        displayed_size = (gdouble) size / 1024;
                        size_string = g_strdup_printf(_("%.0fK"),
displayed_size);
                } else if (size < 1024*1024*1024) {
                        displayed_size = (gdouble) size / (1024*1024);
                        size_string = g_strdup_printf(_("%.0fM"),
displayed_size);
                } else {
                        displayed_size = (gdouble) size / (1024*1024*1024);
                        size_string = g_strdup_printf(_("%.0fG"),
displayed_size);
                }
        }       
        return size_string;
}
somewhat easier to read. imo.

Though, the smoothest way would probably be to have a function that
just did set the lable, having a 2 parameters: The lable to set and
the list of attachments. Then in it you would have had the
size_to_string function sortof. That way you could set the value on
the lable and then free the char*. Right?

Something like the above function but doing it a "static void" instead,
instead of ending it with a "return" you'd have it set the lable. Then
free / clean up ...

> I have an easier solution:
> 
> Convert the size_to_string in to a public static method on
> e-msg-composer-attachment(or -bar) (i.e. just make it an external
> function, say e_msg_composer_attachment_size_to_string).  You dont need
> to change anything but its name (this is more or less what you had in
> the first patch right?).

Um in the first patch, i had a separate function wich was named
"get_attachment_size_string" and returned the appropriate size +
'K'/'M'/'G' in e-msg-composer-attachment-bar.c wich was the reason i
didnt need to re-define the size_to_string() function, it was called
in the get_attachment_size_string() function giving the right return
string.
If i convert size_to_string() in e-msg-composer-attachment-bar.c i'd
really only need to remove the 'static' infront of it. Though,
that might screw things up for other functions using it.. hmm..
> Add a single public method to e-msg-composer-attachment-bar 'get_size'
> which calculates the size of all attachments.
> 
> Then, make the new code use those (fixing leaks).
hm do i get you right? Create two public functions, one in say
e-msg-composer-attachment, that is a copy of the size_to_string()
function, and another one to calculate the total attachment size, that
one in -attachment-bar.

then in e-msg-composer.c i use the above functions in the
attachment_bar_changed_cb() function. Right?

something like this, then, perhaps?!
---- cut ----
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 -u -p -r1.99 e-msg-composer-attachment-bar.c
--- composer/e-msg-composer-attachment-bar.c    23 May 2005 07:10:03
-0000     1.99
+++ composer/e-msg-composer-attachment-bar.c    11 Jul 2005 21:22:24
-0000
@@ -115,8 +115,40 @@ size_to_string (gulong size)
        return size_string;
 }

+gchar *
+e_msg_composer_attachment_size_to_string (gulong size) {
+       gchar *size_string = NULL;
+       gdouble displayed_size;
+
+       if (size < 1024)
+           size_string = NULL;
+       else {
+               if (size < 1024*1024) {
+                       displayed_size = (gdouble) size / 1024;
+                       size_string = g_strdup_printf(_("%.0fK"),
displayed_size);
+               } else if (size < 1024*1024*1024) {
+                       displayed_size = (gdouble) size / (1024*1024);
+                       size_string = g_strdup_printf(_("%.0fM"),
displayed_size);
+               } else {
+                       displayed_size = (gdouble) size /
(1024*1024*1024);
+                       size_string = g_strdup_printf(_("%.0fG"),
displayed_size);
+               }
+       }
+       return size_string;
+}
 /* Attachment handling functions.  */

+gulong
+get_size (EMsgComposerAttachmentBar *bar) {
+       gulong size = 0;
+       GList *p;
+
+       for (p = bar->priv->attachments; p != NULL; p = p->next)
+               size = size + E_MSG_COMPOSER_ATTACHMENT(p->data)->size;
+
+       return size;
+}
+
 static void
 free_attachment_list (EMsgComposerAttachmentBar *bar)
 {
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 -u -p -r1.18 e-msg-composer-attachment-bar.h
--- composer/e-msg-composer-attachment-bar.h    23 May 2005 07:10:03
-0000     1.18
+++ composer/e-msg-composer-attachment-bar.h    11 Jul 2005 21:22:25
-0000
@@ -71,7 +71,8 @@ void e_msg_composer_attachment_bar_attac
 void e_msg_composer_attachment_bar_attach_mime_part
(EMsgComposerAttachmentBar *bar, CamelMimePart *part);
 int e_msg_composer_attachment_bar_get_download_count
(EMsgComposerAttachmentBar *bar);
 void e_msg_composer_attachment_bar_attach_remote_file
(EMsgComposerAttachmentBar *bar,const gchar *url);
-
+gchar *e_msg_composer_attachment_size_to_string (gulong size);
+gulong get_size (EMsgComposerAttachmentBar *bar);

 #ifdef __cplusplus
 }
Index: composer/e-msg-composer.c
===================================================================
RCS file: /cvs/gnome/evolution/composer/e-msg-composer.c,v
retrieving revision 1.512
diff -u -p -r1.512 e-msg-composer.c
--- composer/e-msg-composer.c   23 Jun 2005 09:11:06 -0000      1.512
+++ composer/e-msg-composer.c   11 Jul 2005 21:22:28 -0000
@@ -120,6 +120,7 @@
 #include "mail/em-menu.h"

 #include "e-msg-composer.h"
+#include "e-msg-composer-attachment.h"
 #include "e-msg-composer-attachment-bar.h"
 #include "e-msg-composer-hdrs.h"
 #include "e-msg-composer-select-file.h"
@@ -143,6 +144,14 @@ typedef struct _EMsgComposerPrivate {
        GtkWidget *load;        /* same for load - not used */
 } EMsgComposerPrivate;

+struct _EMsgComposerAttachmentBarPrivate {
+       GtkWidget *attach;      /* attachment file dialogue, if active
*/
+
+       GList *attachments;
+       guint num_attachments;
+       gchar *path;
+};
+
 enum {
        SEND,
        SAVE_DRAFT,
@@ -212,6 +221,7 @@ static void handle_multipart_signed (EMs
 static void set_editor_signature (EMsgComposer *composer);



+
 static EDestination**
 destination_list_to_vector_sized (GList *list, int n)
 {
@@ -2303,10 +2313,14 @@ attachment_bar_changed_cb (EMsgComposerA

        guint attachment_num =
e_msg_composer_attachment_bar_get_num_attachments (
                E_MSG_COMPOSER_ATTACHMENT_BAR
(composer->attachment_bar));
-       if (attachment_num) {
-               gchar *num_text = g_strdup_printf (
-                       ngettext ("<b>%d</b> Attachment", "<b>%d</b>
Attachments", attachment_num),
-                       attachment_num);
+
+       if (attachment_num) {
+               gchar *num_text = g_strdup_printf (
+                       ngettext ("<b>%d</b> Attachment, %s",
+                       "<b>%d</b> Attachments, %s total",
+                       attachment_num),attachment_num,
+
e_msg_composer_attachment_size_to_string(get_size(bar) ));
+
                gtk_label_set_markup (GTK_LABEL
(composer->attachment_expander_num),
                                      num_text);
                g_free (num_text);

-------------
Best regards
/Nicklas