Re: [Evolution-hackers] Patch proposal to e-msg-composer and -attachment-bar
- From: smurfd <smurfd smurfnet homelinux net>
- To: Not Zed <notzed ximian com>
- Cc: evolution-hackers lists ximian com, Jeffrey Stedfast <fejj novell com>
- Subject: Re: [Evolution-hackers] Patch proposal to e-msg-composer and -attachment-bar
- Date: Sun, 03 Jul 2005 22:34:45 +0200
Im abit confused, so does this all mean that the patch is okey as it is.
Or does it mean it's not okey?!
something like this perhaps then, i had to re-use the size_to_string()
function aswell, since it was a private one too.
Though, im only petting in e-msg-composer.c now.
here goes, applied fine for me :
- - - -
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 3 Jul 2005 18:52:54 -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,36 @@ static void handle_multipart_signed (EMs
static void set_editor_signature (EMsgComposer *composer);
+
+static char *
+size_to_string (gulong size)
+{
+ char *size_string;
+
+ /* FIXME: The following should probably go into a separate
module, as
+ we might have to do the same thing in other places as well.
Also,
+ I am not sure this will be OK for all the languages. */
+
+ if (size < 1e3L) {
+ size_string = NULL;
+ } else {
+ gdouble displayed_size;
+
+ if (size < 1e6L) {
+ displayed_size = (gdouble) size / 1.0e3;
+ size_string = g_strdup_printf (_("%.0fK"),
displayed_size);
+ } else if (size < 1e9L) {
+ displayed_size = (gdouble) size / 1.0e6;
+ size_string = g_strdup_printf (_("%.0fM"),
displayed_size);
+ } else {
+ displayed_size = (gdouble) size / 1.0e9;
+ size_string = g_strdup_printf (_("%.0fG"),
displayed_size);
+ }
+ }
+
+ return size_string;
+}
+
static EDestination**
destination_list_to_vector_sized (GList *list, int n)
{
@@ -2303,10 +2342,21 @@ attachment_bar_changed_cb (EMsgComposerA
guint attachment_num =
e_msg_composer_attachment_bar_get_num_attachments (
E_MSG_COMPOSER_ATTACHMENT_BAR
(composer->attachment_bar));
+ GList *p;
+ gulong size = 0;
+
if (attachment_num) {
- gchar *num_text = g_strdup_printf (
- ngettext ("<b>%d</b> Attachment", "<b>%d</b>
Attachments", attachment_num),
- attachment_num);
+ for (p = bar->priv->attachments;
+ p != NULL; p = p->next)
+ size = size +
+ E_MSG_COMPOSER_ATTACHMENT(p->data)->size;
+
+ gchar *num_text = g_strdup_printf (
+ ngettext ("<b>%d</b> Attachment, %s",
+ "<b>%d</b> Attachments, %s total",
+ attachment_num),attachment_num,
+ size_to_string(size));
+
gtk_label_set_markup (GTK_LABEL
(composer->attachment_expander_num),
num_text);
g_free (num_text);
- - - -
On Thu, 2005-06-30 at 11:37 +0800, Not Zed wrote:
> On Mon, 2005-06-27 at 15:47 -0400, Jeffrey Stedfast wrote:
> > 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 dont think this matters a lot. It's all just one big private widget
> anyway (and its not even close to usable outside of the mailer so it
> isn't even very good at being a separate 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.
>
> I dont see why so much of the info is hidden, but the whole
> attachmentbar is awful anyway, so unless it is really fixed it just
> seems a waste of time poking at the edges.
>
> > ... 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.
>
> I missed that the function was in another file, it may as well stay in
> the other file ...
>
>
> > 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
> > >
>
> _______________________________________________
> evolution-hackers maillist - evolution-hackers lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-hackers
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]