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
- Subject: Re: [Evolution-hackers] Patch proposal to e-msg-composer and -attachment-bar
- Date: Sun, 26 Jun 2005 16:40:48 +0200
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) {
> 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);
>
---------------
On Thu, 2005-06-23 at 14:10 +0800, Not Zed wrote:
> Hi there,
>
> To create a diff - either use CVS, i.e. check out the cvs code (anon cvs
> works just fine), make your changes, and then do 'cvs diff composer -Nup
> > file.diff'. This is definitely preferrable and easiest - particularly
> since things change so quickly patches against tar sources are usually
> out of date in no time.
>
> The other alternative is you need to copy the source tree/subtree, have
> one un-touched and one touched version, and then use 'diff -u3 -r
> orig/composer new/composer >file.diff' to create the patch.
>
> On the code, you have way too many unecessary casts (no need to cast a
> gulong to a gulong to add it to a gulong, or cast a function returning
> gchar * to a gchar *, even though the function you're calling actually
> takes a const char * anyway ... and no, dont cast to that either!), you
> leak almost all the strings you allocate, and you're using ngettext
> incorrectly. Dont pass markup to gettext if possible (i guess this case
> is unavoidable), and if you do, make sure you pass simple strings - not
> functions. And certainly dont pass it allocated strings that never get
> freed!
>
> i.e.
> DO!
> text = g_strdup_printf(ngettext("<b>%d</b> Attachment (%s)", "<b>%d</b>
> Attachments (%s total)", attachment_num), attachment_num, size_string)
>
> DONT!
> text = g_strdup_printf(ngettext(g_strdup_printf(""),
> g_strdup_printf(""), n));
>
> AFAICT, apart from leaking like a sieve, the last method can not even
> work properly since the gettext scanner wont be able to find the string
> to use as the translation key, and even if it guessed the quoted string,
> the key is only created at run-time so it couldn't possibly match it to
> anything.
>
> So no ... Srinivasa, the code isn't even close to ok, sorry! Must look
> closer next time!
>
> The idea seems fine though, and the code is easy to fix.
>
> Make sure you make a proper patch once it is ready too.
>
> Michael
>
> PS i dont think you need a separate get_attachment_size_string function
> either, it is only used in that one place and is a small loop that could
> be written in a total of 2 lines of code.
>
> On Wed, 2005-06-22 at 22:24 +0200, smurfd wrote:
> > Hey!
> >
> > I have created a small addition to the attachment bar in the mail
> > composer.
> > What it does, is create a additional string to the "1 Attachment" / "2
> > Attachments" lable in the mail composer attachment bar.
> > Now it says, "1 Attachment, size of 5k" / "2 Attachments, total size of
> > 100k"
> >
> > I Personally think its a very handy thing to have, seeing how some SMTP
> > servers has a limit on how large your mails can be (i was tought
> > recently). If you have N attached files, it can be messy to know how
> > much you have attached to the mail.
> >
> > was unsure how to create a patch for the 3 of the different files, into
> > 1 patch, and since you cant put files on the list, i'll try to describe
> > where the things are supposed to be ;)
> >
> > Best regards
> > /Nicklas
> >
> > --evolution/composer/e-msg-composer-attachment-bar.h--
> > line 74: gchar* get_attachment_size_string (EMsgComposerAttachmentBar
> > *bar);
> > -----
> >
> > --evolution/composer/e-msg-composer-attachment-bar.c--
> > line 404: gchar* get_attachment_size_string (EMsgComposerAttachmentBar
> > *bar) {
> > EMsgComposerAttachmentBarPrivate *priv;
> > GList *p;
> > gulong size=(gulong)0;
> >
> > priv = bar->priv;
> >
> > for (p = priv->attachments; p != NULL; p = p->next) {
> > EMsgComposerAttachment *attachment;
> >
> > attachment = E_MSG_COMPOSER_ATTACHMENT (p->data);
> >
> > size = size + (gulong)attachment->size;
> > }
> > return size_to_string(size);
> > }
> >
> > ----
> >
> > --evolution/composer/e-msg-composer.c--
> > line 2308: if (attachment_num) {
> > gchar *num_text =
> > g_strdup_printf ( ngettext( (gchar *)g_strdup_printf
> > ("<b>%d</b> Attachment, size of %s",(int)attachment_num,
> > get_attachment_size_string((EMsgComposerAttachmentBar *)
> > composer->attachment_bar)),
> >
> > (gchar *)g_strdup_printf
> > ("<b>%d</b> Attachments, total size of %s",
> > (int)attachment_num, get_attachment_size_string(
> > (EMsgComposerAttachmentBar *)composer->attachment_bar))
> > ,attachment_num));
> >
> > gtk_label_set_markup (GTK_LABEL(
> > composer->attachment_expander_num),num_text);
> >
> > g_free (num_text);
> >
> > gtk_widget_show (composer->attachment_expander_icon);
> >
> > } else {
> > ... (same as the old stuff)
> > ----
> >
> > _______________________________________________
> > 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]