Re: [evolution-patches] Mailing list Actions Bounty

On Thu, 2003-12-04 at 19:15, Edgar Antonio Luna Díaz wrote:
Not Zed <notzed ximian com> writes:

>> 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?
Yes, that what I say, I didn't want to modify efh_format_text_header
and efh_format_address but seems that the way.
Well its half there anyway, shouldn't be much to modify.  Actually you already output the address as a mailto, if you fix it up to write out html, it should happen automatically.

> Comments inline.  Might've missed some, its a big patch and i was
> doing a lot of cross referencing.
First thanks a lot for give me that time.

The changes that you aren't commented is because I agreed what you

>> +	if (!em_utils_check_user_can_send_mail ((GtkWidget *) emfv))
>> +		return;

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

Ok, but changing that code to the right position I haven't a parent to
em_utils_check_user_can_send_mail. I set it NULL in this moment,
waiting for your response.
Ahh ok.  Sure may as well stay where you put it.

>> +	if (!(header = camel_medium_get_header (part,
>> +		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.
Indeed, you are right. Here I want to say that this was in a desperate
act of send the patch :), because I didn't used the URI_PARM_* for
what I created them, the idea behind both group of defines is don't
misspell it each time I have to write, just for better error control.

What do you think about it?
Yeah that seems reasonable.

>> +	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,

> You have to convert the text to html (add the various tags).
Yeap yeap.
And if you do that, the popup should work.

> You don't need MLIST_ACTIONS at all, you just have to OR all of the
> possible actions into the popup mask table.
You are right I changed the table to an enormous line now :

>>  	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.

lol, I think that was really reserved, so I must don't touch, now I
EM_POPUP_SELECT_MARK_NOJUNK            = 1<<16,
EM_POPUP_SELECT_MLIST_HELP             = 1<<17,
EM_POPUP_SELECT_LAST                   = 1<<22 /* reserve 2 slots */

Is this ok?
Sure.  On further thought, the reservation idea doesn't really make much sense.  It could just be 1<<21, i.e. the next spot.

> 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.

Yeah, now is with your recommendations. Maybe could be better I'll
show you in next patch.

>> +/* List commands */

>> +#define	LIST_COMMAND_OWNER       "List-Owner"
>> +
>> +#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

I explained this above. But could be removed without any problem.
The em-format-html specific ones could probably go into EMFormatHTML.
Make sure they're namespaced properly (start with the same prefix as other defines in the same header).


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