Re: [evolution-patches] Mailing list Actions Bounty

From: Not Zed <notzed ximian com>
To: Edgar Antonio Luna Díaz <eald linuxuanl org>
Cc: evolution <evolution-patches ximian com>, evolution <evolution-hackers ximian com>
Subject: Re: [evolution-patches] Mailing list Actions Bounty
Date: Wed, 03 Dec 2003 15:25:35 +1100

On Tue, 2003-12-02 at 21:58, Edgar Antonio Luna Díaz wrote:

Hi, 

I appointed me to try this bounty so I did it, even Jorge has
delivered a patch, I had it too advanced to letting in my own computer
only, maybe this could help too.

This patch fill the requirement in this statements:

Gives Options for:
- Unsubscribe
- Subscribe
- Help
- Archives
It consider that maybe could be many url into each header so try for
the first to the last (left to right order), and even
that a header could not exist in the mail, so it don't show the
option.

This options are displayed in:

A sub-menu on Actions menu, and in the popup of mail list.
I could not do it to link on the Preview Message. Just I can't figure
how to send parameter to To: <mailto:here> so it could change the list
of options that will show in the popup menu.

Well you've got some hooks there in em-popup, are you saying they dont work?

The uri management code isn't the best, but I can change it.

Comments inline. Might've missed some, its a big patch and i was
doing a lot of cross referencing.

> Index: camel/camel-mime-message.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/camel/camel-mime-message.c,v
> retrieving revision 1.127
> diff -u -r1.127 camel-mime-message.c
> --- camel/camel-mime-message.c 23 Oct 2003 19:57:58 -0000 1.127
> +++ camel/camel-mime-message.c 2 Dec 2003 10:42:36 -0000
> @@ -76,7 +76,8 @@
> static CamelMimePartClass *parent_class = NULL;
>
> static char *recipient_names[] = {
> - "To", "Cc", "Bcc", "Resent-To", "Resent-Cc", "Resent-Bcc", NULL
> + "To", "Cc", "Bcc", "Resent-To", "Resent-Cc", "Resent-Bcc", "List-Post",
> + NULL
> };

You don't want to do this: list-post is not an rfc822 address header,
it is a url. It may not even necessarily be a mailto: url, from what
i can tell of section 3.4 of the rfc.

> static ssize_t write_to_stream (CamelDataWrapper *data_wrapper, CamelStream *stream);
> Index: camel/camel-mime-message.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/camel/camel-mime-message.h,v
> retrieving revision 1.52
> diff -u -r1.52 camel-mime-message.h
> --- camel/camel-mime-message.h 20 Mar 2003 16:43:26 -0000 1.52
> +++ camel/camel-mime-message.h 2 Dec 2003 10:42:36 -0000
> @@ -44,6 +44,8 @@
> #define CAMEL_RECIPIENT_TYPE_RESENT_CC "Resent-Cc"
> #define CAMEL_RECIPIENT_TYPE_RESENT_BCC "Resent-Bcc"
>
> +#define CAMEL_RECIPIENT_LIST_POST "List-Post"
> +

Again, this shouldn't be added.

> #define CAMEL_MIME_MESSAGE_TYPE     (camel_mime_message_get_type ())
> #define CAMEL_MIME_MESSAGE(obj)     (CAMEL_CHECK_CAST((obj), CAMEL_MIME_MESSAGE_TYPE, CamelMimeMessage))
> #define CAMEL_MIME_MESSAGE_CLASS(k) (CAMEL_CHECK_CLASS_CAST ((k), CAMEL_MIME_MESSAGE_TYPE, CamelMimeMessageClass))
> Index: mail/em-folder-view.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
> retrieving revision 1.14
> diff -u -r1.14 em-folder-view.c
> --- mail/em-folder-view.c 14 Nov 2003 16:20:17 -0000 1.14
> +++ mail/em-folder-view.c 2 Dec 2003 10:42:38 -0000
> @@ -97,6 +97,7 @@
> static void emfv_activate(EMFolderView *emfv, BonoboUIComponent *uic, int state);
>
> static void emfv_message_reply(EMFolderView *emfv, int mode);
> +static void emfv_mailing_list_commands (EMFolderView *emfv, gchar *command);
> static void vfolder_type_current (EMFolderView *emfv, int type);
> static void filter_type_current (EMFolderView *emfv, int type);
>
> @@ -451,6 +452,32 @@
> emfv_message_reply(emfv, REPLY_MODE_LIST);
> }
>
> +/* Mailing_list callbacks */
> +//EMFolderView was before

Use C/C89 comments, not C++/C99 ones.

> +static void
> +emfv_popup_list_help(GtkWidget *w, void *emfv)
> +{
> + emfv_mailing_list_commands (emfv, LIST_COMMAND_HELP);
> +}
> +
> +static void
> +emfv_popup_list_subscribe(GtkWidget *w, void *emfv)
> +{
> + emfv_mailing_list_commands (emfv, LIST_COMMAND_SUBSCRIBE);
> +}
> +
> +static void
> +emfv_popup_list_unsubscribe(GtkWidget *w, void *emfv)
> +{
> + emfv_mailing_list_commands (emfv, LIST_COMMAND_UNSUBSCRIBE);
> +}
> +
> +static void
> +emfv_popup_list_archives(GtkWidget *w, void *emfv)
> +{
> + emfv_mailing_list_commands (emfv, LIST_COMMAND_ARCHIVE);
> +}
> +
> static void
> emfv_popup_reply_all(GtkWidget *w, EMFolderView *emfv)
> {
> @@ -675,8 +702,18 @@
> { EM_POPUP_BAR, "10.emfv" },
> { EM_POPUP_ITEM, "10.emfv.00", N_("_Reply to Sender"), G_CALLBACK(emfv_popup_reply_sender), NULL, "reply.xpm", EM_POPUP_SELECT_ONE },
> { EM_POPUP_ITEM, "10.emfv.01", N_("Reply to _List"), G_CALLBACK(emfv_popup_reply_list), NULL, NULL, EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MAILING_LIST },
> - { EM_POPUP_ITEM, "10.emfv.02", N_("Reply to _All"), G_CALLBACK(emfv_popup_reply_all), NULL, "reply_to_all.xpm", EM_POPUP_SELECT_ONE },
> - { EM_POPUP_ITEM, "10.emfv.03", N_("_Forward"), G_CALLBACK(emfv_popup_forward), NULL, "forward.xpm", EM_POPUP_SELECT_MANY },
> + { EM_POPUP_SUBMENU, "10.emfv.02", N_("List Actions"), NULL, NULL, NULL, EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_ACTIONS},
> + { EM_POPUP_ITEM, "10.emfv.02/00.01", N_("_Request List Help"), G_CALLBACK(emfv_popup_list_help), NULL, NULL,
> + EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_HELP },
> + { EM_POPUP_ITEM, "10.emfv.02/00.02", N_("_Subscribe to list"), G_CALLBACK(emfv_popup_list_subscribe), NULL, NULL,
> + EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_SUBSCRIBE },
> + { EM_POPUP_ITEM, "10.emfv.02/00.03", N_("_Unsubscribe to list"), G_CALLBACK(emfv_popup_list_unsubscribe), NULL, NULL,
> + EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_UNSUBSCRIBE},
> + { EM_POPUP_ITEM, "10.emfv.02/00.01", N_("_View List Archives"), G_CALLBACK(emfv_popup_list_archives), NULL, NULL,
> + EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_ARCHIVES },
> +
> + { EM_POPUP_ITEM, "10.emfv.03", N_("Reply to _All"), G_CALLBACK(emfv_popup_reply_all), NULL, "reply_to_all.xpm", EM_POPUP_SELECT_ONE },
> + { EM_POPUP_ITEM, "10.emfv.04", N_("_Forward"), G_CALLBACK(emfv_popup_forward), NULL, "forward.xpm", EM_POPUP_SELECT_MANY },
>
> { EM_POPUP_BAR, "20.emfv", NULL, NULL, NULL, NULL, EM_POPUP_SELECT_FLAG_FOLLOWUP|EM_POPUP_SELECT_FLAG_COMPLETED|EM_POPUP_SELECT_FLAG_CLEAR },
> { EM_POPUP_ITEM, "20.emfv.00", N_("Follo_w Up..."), G_CALLBACK(emfv_popup_flag_followup), NULL, "flag-for-followup-16.png", EM_POPUP_SELECT_FLAG_FOLLOWUP },
> @@ -1066,6 +1103,19 @@
> /*g_object_unref(clip);*/
> }
>
> +/* Mailing list general function*/
> +static void
> +emfv_mailing_list_commands (EMFolderView *emfv, gchar *command)
> +{
> + if (emfv->list->cursor_uid == NULL)
> + return;
> +
> + if (!em_utils_check_user_can_send_mail ((GtkWidget *) emfv))
> + return;

You only really need to check this for the mailto commands.

> + em_utils_mailing_list_command(emfv->folder, emfv->list->cursor_uid, command);
> +}
> +
> static void
> emfv_message_search(BonoboUIComponent *uic, void *data, const char *path)
> {
> @@ -1224,6 +1274,12 @@
> EMFV_MAP_CALLBACK(emfv_tools_vfolder_mlist, emfv_popup_vfolder_mlist)
> EMFV_MAP_CALLBACK(emfv_tools_vfolder_thread, emfv_popup_vfolder_thread)
>
> +/* Callbacks for Mailing list commans */
> +EMFV_MAP_CALLBACK(emfv_list_help, emfv_popup_list_help)
> +EMFV_MAP_CALLBACK(emfv_list_subscribe, emfv_popup_list_subscribe)
> +EMFV_MAP_CALLBACK(emfv_list_unsubscribe, emfv_popup_list_unsubscribe)
> +EMFV_MAP_CALLBACK(emfv_list_archives, emfv_popup_list_archives)
> +
> /* ********************************************************************** */
>
> static void
> @@ -1298,6 +1354,11 @@
> BONOBO_UI_UNSAFE_VERB ("ViewLoadImages", emfv_view_load_images),
> /* ViewHeaders stuff is a radio */
> /* CaretMode is a toggle */
> + /* Maling List verbs */
> + BONOBO_UI_UNSAFE_VERB ("MailingListHelp", emfv_list_help),
> + BONOBO_UI_UNSAFE_VERB ("MailingListSubscribe", emfv_list_subscribe),
> + BONOBO_UI_UNSAFE_VERB ("MailingListUnsubscribe", emfv_list_unsubscribe),
> + BONOBO_UI_UNSAFE_VERB ("MailingListArchives", emfv_list_archives),
>
> BONOBO_UI_VERB_END
> };
> @@ -1382,6 +1443,11 @@
> { "PrintMessage",             EM_POPUP_SELECT_ONE },
> { "PrintPreviewMessage",      EM_POPUP_SELECT_ONE },
>
> + { "ListCommands",             EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_ACTIONS },
> + { "MailingListHelp",          EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_HELP },
> + { "MailingListSubscribe",     EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_SUBSCRIBE },
> + { "MailingListUnsubscribe",   EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_UNSUBSCRIBE },
> + { "MailingListArchives",      EM_POPUP_SELECT_ONE|EM_POPUP_SELECT_MLIST_ARCHIVES },
> { "TextZoomIn",       EM_POPUP_SELECT_ONE },
> { "TextZoomOut",       EM_POPUP_SELECT_ONE },
> { "TextZoomReset",       EM_POPUP_SELECT_ONE },
> @@ -2013,3 +2079,4 @@
> emfv, NULL, NULL);
> g_object_unref(gconf);
> }
> +
> Index: mail/em-format-html.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-format-html.c,v
> retrieving revision 1.14
> diff -u -r1.14 em-format-html.c
> --- mail/em-format-html.c 24 Nov 2003 01:38:11 -0000 1.14
> +++ mail/em-format-html.c 2 Dec 2003 10:42:39 -0000
> @@ -1452,6 +1452,37 @@
> }
>
> static void
> +efh_format_list_post(EMFormat *emf, CamelStream *stream, const CamelInternetAddress *cia, CamelMedium *part, guint32 flags)
> +{
> + const gchar *text, *list_post, *header;
> + CamelURL *url;
> +
> + if (cia == NULL || !camel_internet_address_get(cia, 0, NULL, NULL))
> + return;
> +
> + list_post = camel_address_format((CamelAddress *)cia);
> + list_post = g_strdup_printf("mailto:%s", list_post);
> + url = "" NULL);
> +
> + if (!(header = camel_medium_get_header (part, URI_PARAM_LIST_HELP)))
> + camel_url_set_param(url, "x-list-help", header);

> + if (!(header = camel_medium_get_header (part, URI_PARAM_LIST_ARCHIVE)))
> + camel_url_set_param(url, "x-list-archive", header);
> +
> + if (!(header = camel_medium_get_header (part, URI_PARAM_LIST_SUBSCRIBE)))
> + camel_url_set_param(url, "x-list-subscribe", header);
> +
> + if (!(header = camel_medium_get_header (part, URI_PARAM_LIST_UNSUBSCRIBE)))
> + camel_url_set_param(url, "x-list-unsubscribe", header);

^^ use a table. Besides, you're using the wrong defines, you're after
LIST_COMMAND_ ones in medium_get_header, and URI_PARAM_LIST* in
url_set_param.

> + text = camel_url_to_string(url, 0);
> + camel_url_free(url);
> + g_print("url: %s\n",text);
> + efh_format_text_header(emf, stream, _("List Post"), text,
> flags | EM_FORMAT_HTML_HEADER_HTML);

You have to convert the text to html (add the various tags).

> +}
> +
> +static void
> efh_format_header(EMFormat *emf, CamelStream *stream, CamelMedium *part, const char *namein, guint32 flags, const char *charset)
> {
> #define msg ((CamelMimeMessage *)part)
> @@ -1461,18 +1492,21 @@
> name = alloca(strlen(namein)+1);
> strcpy(name, namein);
> camel_strdown(name);
> -
> if (!strcmp(name, "from"))
> efh_format_address(emf, stream, camel_mime_message_get_from(msg), _("From"), flags);
> else if (!strcmp(name, "reply-to"))
> efh_format_address(emf, stream, camel_mime_message_get_reply_to(msg), _("Reply-To"), flags);
> else if (!strcmp(name, "to"))
> - efh_format_address(emf, stream, camel_mime_message_get_recipients(msg, CAMEL_RECIPIENT_TYPE_TO), _("To"), flags);
> + efh_format_address(emf, stream, camel_mime_message_get_recipients(msg, CAMEL_RECIPIENT_TYPE_TO), _("To"), flags);
> else if (!strcmp(name, "cc"))
> efh_format_address(emf, stream, camel_mime_message_get_recipients(msg, CAMEL_RECIPIENT_TYPE_CC), _("Cc"), flags);
> else if (!strcmp(name, "bcc"))
> efh_format_address(emf, stream, camel_mime_message_get_recipients(msg, CAMEL_RECIPIENT_TYPE_BCC), _("Bcc"), flags);
> - else {
> + else if (!strcmp(name, "list-post")) {
> + /* Mailing list do this ensure ?*/
> + if (camel_medium_get_header (part, "List-Post"))
> + efh_format_list_post(emf, stream,
> camel_mime_message_get_recipients(msg, CAMEL_RECIPIENT_LIST_POST),
> part, flags);

it aint an address, so you can't pass it around inside a
camelinternetaddreses, etc.

> + } else {
> const char *txt, *label;
> char *value = NULL;
>
> Index: mail/em-popup.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-popup.c,v
> retrieving revision 1.7
> diff -u -r1.7 em-popup.c
> --- mail/em-popup.c 12 Nov 2003 21:12:59 -0000 1.7
> +++ mail/em-popup.c 2 Dec 2003 10:42:40 -0000
> @@ -22,8 +22,13 @@
> #include <camel/camel-folder.h>
> #include <camel/camel-mime-message.h>
> #include <camel/camel-string-utils.h>
> +#include <camel/camel-url.h>
>
> static void emp_standard_menu_factory(EMPopup *emp, EMPopupTarget *target, void *data);
> +guint32 em_mailing_list_mask_headers (CamelMimeMessage *message, guint32 *rmask);
> +guint32 em_mailing_list_set_mask(CamelFolder *folder, const gchar *uid, guint32 *rmask);
> +guint32 em_uri_mailing_list_set_mask(CamelURL *url, guint32 *rmask);
> +static void emp_list_uri_by_action(gchar *uri, const gchar *param_name);
>
> struct _EMPopupFactory {
> struct _EMPopupFactory *next, *prev;
> @@ -476,8 +481,10 @@
>
> if (i == 0 && uids->len == 1
>     && (tmp = camel_message_info_mlist(info))
> -     && tmp[0] != 0)
> +     && tmp[0] != 0) {
> mask &= ~EM_POPUP_SELECT_MAILING_LIST;
> + em_mailing_list_set_mask(folder, uids->pdata[i], &mask);
> + }
>
> camel_folder_free_message_info(folder, info);
> }
> @@ -487,6 +494,76 @@
> return t;
> }
>
> +guint32
> +em_mailing_list_mask_headers (CamelMimeMessage *message, guint32
> *rmask)

This is internal, don't export it from the module.

> +{
> + CamelMedium *medium = (CamelMedium *)message;
> + if(camel_medium_get_header (medium, "List-Help")) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_HELP;
> + }
> +
> + if(camel_medium_get_header (medium, "List-Subscribe")) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_SUBSCRIBE;
> + }
> +
> + if(camel_medium_get_header (medium, "List-Unsubscribe")) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_UNSUBSCRIBE;
> + }
> +
> + if(camel_medium_get_header (medium, "List-Archive")) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_ARCHIVES;
> + }
> +
> + if(camel_medium_get_header (medium, "List-Owner")) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_OWNER;
> + }

^^ this should be driven from a table. e.g.
struct {
const char *name;
guint32 mask;
} table[] = {
{ "List-Help", EM_POPOP_SELECT_MLIST_HELP },
... etc
};

for (i=0;i<length(table);i++ {
if get header table[x].name, and header is parsable
    unset the mask in table[x].mask
}

> + if (((EM_POPUP_SELECT_MLIST_OWNER
> +      | EM_POPUP_SELECT_MLIST_ARCHIVES
> +      | EM_POPUP_SELECT_MLIST_SUBSCRIBE
> +      | EM_POPUP_SELECT_MLIST_UNSUBSCRIBE
> +      | EM_POPUP_SELECT_MLIST_HELP) & *rmask) > 0) {
> + *rmask &= ~EM_POPUP_SELECT_MLIST_ACTIONS;
> + }

You don't need MLIST_ACTIONS at all, you just have to OR all of the
possible actions into the popup mask table.

> + return *rmask > 0;
> +}

I think this should take and return the mask, not a pointer. No need
to return a boolean (which is mistyped in the prototype).

> +
> +guint32
> +em_mailing_list_set_mask(CamelFolder *folder, const gchar *uid, guint32 *rmask)
> +{
> + CamelMimeMessage *message = camel_folder_get_message ( folder, uid, NULL);
> + return em_mailing_list_mask_headers (message, rmask);
> +}
> +
> +guint32
> +em_uri_mailing_list_set_mask(CamelURL *url, guint32 *rmask)
> +{
> + if(camel_url_get_param (url, "List-Help")) {
> + *rmask &= ~EM_POPUP_URI_LIST_HELP;
> + }
> +
> + if(camel_url_get_param (url, "List-Subscribe")) {
> + *rmask &= ~EM_POPUP_URI_LIST_SUBSCRIBE;
> + }
> +
> + if(camel_url_get_param (url, "List-Unsubscribe")) {
> + *rmask &= ~EM_POPUP_URI_LIST_UNSUBSCRIBE;
> + }
> +
> + if(camel_url_get_param (url, "List-Archive")) {
> + *rmask &= ~EM_POPUP_URI_LIST_ARCHIVES;
> + }
> +
> + if(camel_url_get_param (url, "List-Owner")) {
> + *rmask &= ~EM_POPUP_URI_LIST_OWNER;
> + }
> +
> + return *rmask > 0;
> +
> +}

similar comments to above. use a table, and return the value, dont
use a pointer.

> EMPopupTarget *
> em_popup_target_new_uri(const char *uri)
> {
> @@ -499,9 +576,13 @@
> if (g_ascii_strncasecmp(uri, "http:", 5) == 0
>     || g_ascii_strncasecmp(uri, "https:", 6) == 0)
> mask &= ~EM_POPUP_URI_HTTP;
> - if (g_ascii_strncasecmp(uri, "mailto:", 7) == 0)
> + if (g_ascii_strncasecmp(uri, "mailto:", 7) == 0) {
> + CamelURL * url;
> mask &= ~EM_POPUP_URI_MAILTO;
> - else
> + url = "" NULL);
> + em_uri_mailing_list_set_mask(url, &mask);
> + // eald here pending set mask
> + } else
> mask &= ~EM_POPUP_URI_NOT_MAILTO;
>
> t->mask = mask;
> @@ -509,6 +590,7 @@
> return t;
> }
>
> +
> EMPopupTarget *
> em_popup_target_new_part(struct _CamelMimePart *part, const char *mime_type)
> {
> @@ -553,6 +635,8 @@
> camel_object_unref(t->data.part.part);
> g_free(t->data.part.mime_type);
> break;
> + case EM_POPUP_TARGET_FOLDER:
> + break;
> }
>
> g_free(t);
> @@ -681,10 +765,53 @@
> printf("UNIMPLEMENTED: Add address '%s'\n", t->data.uri);
> }
>
> +static void
> +emp_popup_mailing_list_help(GtkWidget *w, EMPopupTarget *t)
> +{
> + emp_list_uri_by_action(t->data.uri, URI_PARAM_LIST_HELP);
> +}
> +
> +static void
> +emp_popup_mailing_list_subscribe(GtkWidget *w, EMPopupTarget *t)
> +{
> + emp_list_uri_by_action(t->data.uri, URI_PARAM_LIST_UNSUBSCRIBE);
> +}
> +
> +static void
> +emp_popup_mailing_list_unsubscribe(GtkWidget *w, EMPopupTarget *t)
> +{
> + emp_list_uri_by_action(t->data.uri, URI_PARAM_LIST_UNSUBSCRIBE);
> +}
> +
> +static void
> +emp_popup_mailing_list_archives(GtkWidget *w, EMPopupTarget *t)
> +{
> + emp_list_uri_by_action(t->data.uri, URI_PARAM_LIST_ARCHIVE);
> +}
> +
> +static void
> +emp_list_uri_by_action(gchar *uri, const gchar *param_name)
> +{
> + CamelURL *url;
> + const gchar *param;
> + url = "" NULL);
> + param = camel_url_get_param(url, param_name);
> + mailing_list_run_action(param);
> + camel_url_free(url);
> +}

define this before ethe code that uses it so you don't have to maintain
a separate forward declaration.

again use c types, not gtypes for the basics.

> static EMPopupItem emp_standard_uri_popups[] = {
> { EM_POPUP_ITEM, "00.uri.00", N_("_Open Link in Browser"), G_CALLBACK(emp_uri_popup_link_open), NULL, NULL, EM_POPUP_URI_NOT_MAILTO },
> { EM_POPUP_ITEM, "00.uri.10", N_("Se_nd message to..."), G_CALLBACK(emp_uri_popup_address_send), NULL, NULL, EM_POPUP_URI_MAILTO },
> { EM_POPUP_ITEM, "00.uri.20", N_("_Add to Addressbook"), G_CALLBACK(emp_uri_popup_address_add), NULL, NULL, EM_POPUP_URI_MAILTO },
> + { EM_POPUP_ITEM, "00.uri.00", N_("_Request List Help"), G_CALLBACK(emp_popup_mailing_list_help), NULL, NULL,
> + EM_POPUP_URI_MAILTO|EM_POPUP_URI_LIST_HELP },
> + { EM_POPUP_ITEM, "00.uri.01", N_("_Subscribe to list"), G_CALLBACK(emp_popup_mailing_list_subscribe), NULL, NULL,
> + EM_POPUP_URI_MAILTO|EM_POPUP_URI_LIST_SUBSCRIBE },
> + { EM_POPUP_ITEM, "00.uri.02", N_("_Unsubscribe to list"), G_CALLBACK(emp_popup_mailing_list_unsubscribe), NULL, NULL,
> + EM_POPUP_URI_MAILTO|EM_POPUP_URI_LIST_UNSUBSCRIBE},
> + { EM_POPUP_ITEM, "00.uri.03", N_("_View List Archives"), G_CALLBACK(emp_popup_mailing_list_archives), NULL, NULL,
> + EM_POPUP_URI_MAILTO|EM_POPUP_URI_LIST_ARCHIVES }
> };
>
> /* ********************************************************************** */
> Index: mail/em-popup.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-popup.h,v
> retrieving revision 1.4
> diff -u -r1.4 em-popup.h
> --- mail/em-popup.h 12 Nov 2003 21:12:59 -0000 1.4
> +++ mail/em-popup.h 2 Dec 2003 10:42:40 -0000
> @@ -69,35 +69,46 @@
> EM_POPUP_TARGET_SELECT,
> EM_POPUP_TARGET_URI,
> EM_POPUP_TARGET_PART,
> - EM_POPUP_TARGET_FOLDER,
> + EM_POPUP_TARGET_FOLDER
> };
>
> /* Flags that describe a TARGET_SELECT */
> enum {
> - EM_POPUP_SELECT_ONE     = 1<<1,
> - EM_POPUP_SELECT_MANY     = 1<<2,
> + EM_POPUP_SELECT_ONE        = 1<<1,
> + EM_POPUP_SELECT_MANY        = 1<<2,
> EM_POPUP_SELECT_MARK_READ              = 1<<3,
> EM_POPUP_SELECT_MARK_UNREAD            = 1<<4,
> EM_POPUP_SELECT_DELETE                 = 1<<5,
> EM_POPUP_SELECT_UNDELETE               = 1<<6,
> - EM_POPUP_SELECT_MAILING_LIST            = 1<<7,
> + EM_POPUP_SELECT_MAILING_LIST           = 1<<7,
> EM_POPUP_SELECT_RESEND                 = 1<<8,
> EM_POPUP_SELECT_MARK_IMPORTANT         = 1<<9,
> EM_POPUP_SELECT_MARK_UNIMPORTANT       = 1<<10,
> - EM_POPUP_SELECT_FLAG_FOLLOWUP      = 1<<11,
> + EM_POPUP_SELECT_FLAG_FOLLOWUP          = 1<<11,
> EM_POPUP_SELECT_FLAG_COMPLETED         = 1<<12,
> EM_POPUP_SELECT_FLAG_CLEAR             = 1<<13,
> EM_POPUP_SELECT_ADD_SENDER             = 1<<14,
> EM_POPUP_SELECT_MARK_JUNK              = 1<<15,
> EM_POPUP_SELECT_MARK_NOJUNK            = 1<<16,
> - EM_POPUP_SELECT_LAST = 1<<18 /* reserve 2 slots */
> + EM_POPUP_SELECT_LAST = 1<<18, /* reserve 2 slots */

The reservations are supposed to be at the end, not in the middle :)
They're not actually very useful anyway.

> + EM_POPUP_SELECT_MLIST_ACTIONS          = 1<<19,
> + EM_POPUP_SELECT_MLIST_HELP             = 1<<20,
> + EM_POPUP_SELECT_MLIST_SUBSCRIBE        = 1<<21,
> + EM_POPUP_SELECT_MLIST_UNSUBSCRIBE      = 1<<22,
> + EM_POPUP_SELECT_MLIST_ARCHIVES         = 1<<23,
> + EM_POPUP_SELECT_MLIST_OWNER            = 1<<24
> };
>
> /* Flags that describe a TARGET_URI */
> enum {
> - EM_POPUP_URI_HTTP = 1<<0,
> - EM_POPUP_URI_MAILTO = 1<<1,
> - EM_POPUP_URI_NOT_MAILTO = 1<<2,
> + EM_POPUP_URI_HTTP              = 1<<0,
> + EM_POPUP_URI_MAILTO            = 1<<1,
> + EM_POPUP_URI_NOT_MAILTO        = 1<<2,
> + EM_POPUP_URI_LIST_HELP         = 1<<3,
> + EM_POPUP_URI_LIST_SUBSCRIBE    = 1<<4,
> + EM_POPUP_URI_LIST_UNSUBSCRIBE = 1<<5,
> + EM_POPUP_URI_LIST_ARCHIVES     = 1<<6,
> + EM_POPUP_URI_LIST_OWNER        = 1<<7
> };
>
> /* Flags that describe TARGET_PART */
> Index: mail/em-utils.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.c,v
> retrieving revision 1.8
> diff -u -r1.8 em-utils.c
> --- mail/em-utils.c 19 Nov 2003 21:22:12 -0000 1.8
> +++ mail/em-utils.c 2 Dec 2003 10:42:41 -0000
> @@ -30,6 +30,7 @@
> #include <sys/stat.h>
> #include <errno.h>
> #include <time.h>
> +#include <gnome.h>
>
> #include <camel/camel-stream-fs.h>
> #include <camel/camel-url-scanner.h>
> @@ -1242,6 +1243,80 @@
> e_msg_composer_unset_changed (composer);
> }
>
> +/* List commands */
> +
> +void
> +mailing_list_action (CamelMimeMessage *message, const gchar *header)
> +{
> + const gchar *action;
> +
> + if (!(action = "" ((CamelMedium *) message, header)))
> + return;
> +
> + mailing_list_run_action(action);
> +}

^^ i don't think this is necessary, the 1 line of logic can be placed
in the one caller of this.

the following function has a lot of problems. if exported, fix the name.
if written right it should only be a handful lines of code.

> +void
> +mailing_list_run_action(const gchar *action)
> +{
> + const gchar *p, *url;
> + gchar **uris;
> + gint i;
> + GError *err = NULL;
> +#define n 5

ouch, if you mean 5 just use 5. actually you don't mean five either.

> + /* 5 elements at my descretion */
> + uris = g_strsplit(action,",", n);

> + /* This code is almost ready to show a list of possible
> + * actions if that is decided to use. */
> + for(i = 0; i < n; i++) {

you should do

for (i=0; uris[i]; i++)

so you dont overrun the string array.

> + action = "">
why strdup the uri? you never modify it even, and you alreayd have a
copy of it.

> + p = action;
> + while (*action == ' ' || *action == '\t' || *action == '<')
> + action++;

this should check the next char is '<' otherwise it should finish. it
also wont handle multiple actions with comments in between. maybe
strchr '<' would be sufficient.

> + if (!(strncmp (action, "mailto:", 6))) {

you are checking 7 chars here. use g_ascii_strcasecmp() too.

> + while (*p && !strchr (">", *p))
> + p++;

umm, perhaps you want:
   p = strchr(action, '>')
   if (p == NULL || p == action)
      continue;

(this also covers the strlen==0 case)

> + url = "" (action, p - action);

since you hve a copy of the string already, just do *p++ = 0 to
terminate it, and use that.

> + if (strlen (url) == 0)
> + continue;
> + em_utils_compose_new_message_with_mailto (url);
> + break;
> + } else {
> + while (*p && !strchr (">", *p))
> + p++;

to avoid this duplication, parse the <...> block first, then check the
protocol inside of it.

> + url = "" (action, p - action);
> + if (strlen (url) == 0)
> + continue;
> + gnome_url_show (url, &err);
> + if (err) {
> + g_warning ("gnome_url_show: %s", err->message);
> + g_error_free(err);
> + } else {
> + break;
> + }
> + }
> + }
> +#undef n
> + return;
> +}

you dont free the string array, and leak every action you strdup'd earlier.

> +void
> +em_utils_mailing_list_action_folder (CamelFolder *folder, const gchar *uid, CamelMimeMessage *message, void *user_data)
> +{
> + gchar *command = (gchar *)user_data;
> +
> + mailing_list_action (message, command);
> +}

^^ do not export this, it is an internal callback.

> +void
> +em_utils_mailing_list_command (CamelFolder *folder, const char *uid, gchar * command)
> +{
> + mail_get_message (folder, uid,
> + em_utils_mailing_list_action_folder, GINT_TO_POINTER(command), mail_thread_new);
> +}

Bug here. Command is a char * not an int.

> /**
>   * em_utils_post_reply_to_message_by_uid:
>   * @folder: folder containing message to reply to
> Index: mail/em-utils.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.h,v
> retrieving revision 1.4
> diff -u -r1.4 em-utils.h
> --- mail/em-utils.h 27 Oct 2003 21:31:19 -0000 1.4
> +++ mail/em-utils.h 2 Dec 2003 10:42:41 -0000
> @@ -80,6 +80,26 @@
> REPLY_MODE_LIST
> };
>
> +/* List commands */
> +#define LIST_COMMAND_HELP        "List-Help"
> +#define LIST_COMMAND_SUBSCRIBE   "List-Subscribe"
> +#define LIST_COMMAND_UNSUBSCRIBE "List-Unsubscribe"
> +#define LIST_COMMAND_ARCHIVE     "List-Archive"
> +#define LIST_COMMAND_OWNER       "List-Owner"
> +
> +#define URI_PARAM_LIST_HELP        "x-list-help"
> +#define URI_PARAM_LIST_SUBSCRIBE   "x-list-subscribe"
> +#define URI_PARAM_LIST_UNSUBSCRIBE "x-list-unsubscribe"
> +#define URI_PARAM_LIST_ARCHIVE     "x-list-archive"

I'm not sure these defines are needed. The last ones are used in em-format-html

> +typedef enum _mailing_list_command mailing_list_command;

This enum isn't used.

> +void mailing_list_action (struct _CamelMimeMessage *message, const char *header);
> +void mailing_list_commands (struct _CamelFolder *folder, const char *uid, struct _CamelMimeMessage *message, void *user_data);
> +void em_utils_mailing_list_command (struct _CamelFolder *folder, const char *uid, char *command);
> +void mailing_list_run_action(const char *url);
> +void em_utils_mailing_list_action_folder (struct _CamelFolder *folder, const char *uid, struct _CamelMimeMessage *message, void *user_data) ;

Anything exported from em-utils.c should start with em_utils_.

A bunch of those above (all the mailing_list_* functions) seem to only
be internal, and should be defined static in the .c file and not in
the .h one. They should also start with 'emu_' or probably 'emuml_' or
something.

> void em_utils_reply_to_message (struct _CamelMimeMessage *message, int mode);
> void em_utils_reply_to_message_by_uid (struct _CamelFolder *folder, const char *uid, int mode);
>
> Index: ui/evolution-mail-message.xml
> ===================================================================
> RCS file: /cvs/gnome/evolution/ui/evolution-mail-message.xml,v
> retrieving revision 1.52
> diff -u -r1.52 evolution-mail-message.xml
> --- ui/evolution-mail-message.xml 12 Nov 2003 21:13:05 -0000 1.52
> +++ ui/evolution-mail-message.xml 2 Dec 2003 10:42:42 -0000
> @@ -326,6 +326,14 @@
>
> <separator f="" name="emaillist2"/>
>
> +          <submenu name="ListCommands" _label="L_ist">
> +            <menuitem verb="MailingListHelp" _label="_Request List Help"/>
> +            <menuitem verb="MailingListSubscribe" _label="_Subscribe to list"/>
> +            <menuitem verb="MailingListUnsubscribe" _label="_Unsubscribe from list"/>
> +            <separator f="" name="listlist1"/>
> +            <menuitem verb="MailingListArchives" _label="_View List Archives"/>
> +          </submenu>
> +
>            <menuitem name="MessageReplySender" verb="" _label="_Reply to Sender"/>
>            <menuitem name="MessageReplyList" verb="" _label="Reply to _List"/>
>            <menuitem name="MessageReplyAll" verb="" _label="Reply to _All"/>
>

Follow-Ups:
- Re: [evolution-patches] Mailing list Actions Bounty
  - From: Edgar Antonio Luna Díaz

References:
- [evolution-patches] Mailing list Actions Bounty
  - From: Edgar Antonio Luna Díaz

[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]