[evolution-patches] Re: [Evolution-hackers] Post-to header implementation



Hello,

Allright... Still quite some improvements to be done, but the one-liners that were mentioned are fixed now:

   * http://home.wanadoo.nl/meilof/evolution-nntp-patch-17

Since this is all I'm likely to produce today (going to eat now), I think this can be applied to CVS?


Meilof
-- meilof wanadoo nl

Not Zed wrote:

@@ -285,14 +484,26 @@
static void
nntp_folder_class_init (CamelNNTPFolderClass *camel_nntp_folder_class) {
+	CamelDiscoFolderClass *camel_disco_folder_class = CAMEL_DISCO_FOLDER_CLASS (camel_nntp_folder_class);
	CamelFolderClass *camel_folder_class = CAMEL_FOLDER_CLASS (camel_nntp_folder_class);

-	parent_class = CAMEL_FOLDER_CLASS (camel_type_get_global_classfuncs (camel_folder_get_type ()));
+	parent_class = CAMEL_DISCO_FOLDER_CLASS (camel_type_get_global_classfuncs (camel_disco_folder_get_type ()));
+	folder_class = CAMEL_FOLDER_CLASS (camel_type_get_global_classfuncs (camel_folder_get_type ()));

This is a hangover from older code, you only need to call

folder_class = (CamelFolderClass *)camel_folder_get_type();

Actually you only need to get the parent class anyway,
CameDiscoFolderClass.  You shouldn't access CamelFolderClass that way.

So I do

disco_folder_class = (CamelDiscoFolderClass *)camel_disco_folder_get_type();

as well? And how should I override CamelFolderClass functions then?

camel_nntp_folder_new (or even camel_nntp_folder_construct)
also needs to setup the persistent metadata file on the folder object.

Call it ("%s.cmeta", nntp_folder->storage_path)

Set it and load it using:

camel_object_set(folder, NULL, CAMEL_OBJECT_STATE_FILE, path, NULL);
camel_object_state_read(folder);

And in the sync online/offline methods you need to call
camel_object_state_write(folder)

This is going to take some testing, so not for today.

+nntp_connect_offline(CamelService *service, CamelException *ex)
+{
+	CamelNNTPStore *nntp_store = CAMEL_NNTP_STORE(service);
+	CamelDiscoStore *disco_store = (CamelDiscoStore*) nntp_store;
+	char *path;
+
+	if (nntp_store->storage_path == NULL)
+		return FALSE;
+
+	/* setup store-wide cache */
+	if (nntp_store->cache == NULL) {
+		nntp_store->cache = camel_data_cache_new (nntp_store->storage_path,
0, ex);
+		if (nntp_store->cache == NULL)
+			return FALSE;

It should probably still be able to operate with no cache.

Waiting do do this as well.

+nntp_folder_info_from_store_info(gboolean short_notation, CamelURL *base_url, CamelStoreInfo *si)

Should have storeinfo as the first argument, followed by base_url and
short_notation.

You should probably just pass in the store as the first arg actually,
with no url and no short_notation, since you can get them both from
the store pointer.

This doesn't go for short_notation, because we specifically disable it for the subscription dialog...

+static CamelFolderInfo *
+nntp_store_get_subscribed_folder_info(CamelNNTPStore *store, const
char *top, guint flags, CamelException *ex)
+{
+	CamelURL *url = CAMEL_SERVICE (store)->url;
+	int i;
+	CamelStoreInfo *si;
+	CamelFolderInfo *first = NULL, *last = NULL, *fi = NULL;
+
+	/* since we do not do a tree, any request that is not for root is
sure to give no results */
+	if (top != NULL && top[0] != 0) return NULL;

You should actually be able to do get_folder_info("news.group") and get
back 'news.group' (if it exists).

Waiting with this.

+				   the list as a placeholder */
+				if (!last ||
+				    strncasecmp(si->path, last->full_name, (len = strlen(last-
full_name))) ||
+				    si->path[len] != '.') {

Since len is only used in that else block you should probably make it
local to the block that includes it, and calculate it separately to
the if expression.

It has to be done within the if, because otherwise you're not sure "last" exists. I could make it another if, but I doubt whether that would actually improve readability.

and a newline after an if expression or open brace, always.

After an open brace? Like:

+	if (online && (top == NULL || top[0] == 0)) {

+		/* we may need to update */
+		if (summary->last_newslist[0] != 0) {

+			char date[14];



@@ -505,7 +1038,7 @@

	if (!nntp_connected (store, NULL))
		return -1;
-	
+
	/* Check for unprocessed data, ! */
	if (store->stream->mode == CAMEL_NNTP_STREAM_DATA) {
		g_warning("Unprocessed data left in stream, flushing");
@@ -514,6 +1047,7 @@
	}
	camel_nntp_stream_set_mode(store->stream, CAMEL_NNTP_STREAM_LINE);

+command_begin_send:
	va_start(ap, fmt);
	ps = p = fmt;
	while ( (c = *p++) ) {
@@ -557,7 +1091,22 @@
	camel_stream_write((CamelStream *)store->mem, ps, p-ps-1);
	dd(printf("NNTP_COMMAND: '%.*s'\n", (int)store->mem->buffer->len,
store->mem->buffer->data));
	camel_stream_write((CamelStream *)store->mem, "\r\n", 2);
-	camel_stream_write((CamelStream *)store->stream, store->mem->buffer-
data, store->mem->buffer->len);
+
+	if (camel_stream_write((CamelStream *)store->stream, store->mem-
buffer->data, store->mem->buffer->len) == -1 && errno != EINTR) {
+		camel_stream_reset((CamelStream *)store->mem);

+		/* FIXME: hack */
+		g_byte_array_set_size(store->mem->buffer, 0);
+

instead of doing this, command_begin_send could be just beofre the if
(camel_stream_write()) call, 2 lines above.

Actually the whole lot should just be a while () loop with no
command_begin_send, since the content of the command string (the mem
stream) will not change.

The array_set_size call is required if you exit though.  Or better,
move camel_stream_reset(mem); and g_byte_array_set_size(store->mem, 0)
to where you currently have command_begin_send label, so its always
reset before a new command is created.

+reconnect:
+		/* some error, re-connect */
+		camel_service_disconnect (CAMEL_SERVICE (store), FALSE, NULL);
+
+		if (!nntp_connected(store, NULL))
+			return -1;
+
+		goto command_begin_send;
+	}
+
	camel_stream_reset((CamelStream *)store->mem);
	/* FIXME: hack */
	g_byte_array_set_size(store->mem->buffer, 0);

^^ i.e. move this one, remove the other one.

+CamelNNTPStoreInfo *
+camel_nntp_store_summary_add_from_full(CamelNNTPStoreSummary *s, const char *full, char dir_sep)


^^ this stuff is imap specific, should either be removed or changed for
nntp.

Yeah I know. Will clean it up.

static void
headers_set_sensitivity (EMsgComposerHdrs *h)
{
-	bonobo_ui_component_set_prop (
+/*	bonobo_ui_component_set_prop (
		h->priv->uic, "/commands/ViewFrom", "sensitive",
-		h->visible_mask & E_MSG_COMPOSER_VISIBLE_FROM ? "1" : "0", NULL);
-	
+		h->visible_mask & E_MSG_COMPOSER_VISIBLE_FROM ? "1" : "0", NULL);*/
+
+	/* these ones are always on */
+	bonobo_ui_component_set_prop (
+		h->priv->uic, "/commands/ViewTo", "sensitive",
+		h->visible_mask & E_MSG_COMPOSER_VISIBLE_TO ? "0" : "1", NULL);
+
	bonobo_ui_component_set_prop (
+		h->priv->uic, "/commands/ViewPostTo", "sensitive",
+		h->visible_mask & E_MSG_COMPOSER_VISIBLE_POSTTO ? "0" : "1", NULL);
+
+/*	bonobo_ui_component_set_prop (
		h->priv->uic, "/commands/ViewReplyTo", "sensitive",
		h->visible_mask & E_MSG_COMPOSER_VISIBLE_REPLYTO ? "1" : "0",
NULL);
	
@@ -625,7 +704,7 @@
	
	bonobo_ui_component_set_prop (
		h->priv->uic, "/commands/ViewBCC", "sensitive",
-		h->visible_mask & E_MSG_COMPOSER_VISIBLE_BCC ? "1" : "0", NULL);
+		h->visible_mask & E_MSG_COMPOSER_VISIBLE_BCC ? "1" : "0", NULL); */

Do you really mean to comment out this block of code?

Yeah, it caused the fields to be blacked out for posting...

+{
+	/* compile the name */
+	char *caption, *tmp, *tmp2;
+	gboolean post_custom;
+
+	if (hdrs->priv->post_to.entry == NULL) return;
+
+	caption = g_strdup("");
+
+	while (urls) {
+		tmp = folder_name_to_string(hdrs, (char *)urls->data);

How does this work with imap/other store's folder names?

Just copied it from the selection button, IIRC. Will look at it...

+		if (tmp) {
+			tmp2 = g_strconcat(caption, ", ", tmp, NULL);
+			g_free(caption);
instead of this append + free stuff, use a g_string to build the
result (equivalent to java's StringBuffer vs using String's).

Allright, I'll do that later, too.

+			caption = tmp2;
+			g_free(tmp);
+		}
+		urls = g_list_next(urls);
+	}
+
+	post_custom = hdrs->priv->post_custom;
+	gtk_entry_set_text(GTK_ENTRY(hdrs->priv->post_to.entry), caption
[0] ? caption + 2 : "");
+	hdrs->priv->post_custom = post_custom;
+	g_free(caption);
+}
+
+void
+e_msg_composer_hdrs_set_post_to_base (EMsgComposerHdrs *hdrs,
+                                      const char *base, const char
*post_to)
+{
+	GList *lst, *curlist;
+	char *hdr_copy = g_strdup(post_to), *caption, *tmp, *tmp2;
+	gboolean post_custom;
+
+	/* split to newsgroup names */
+	lst = newsgroups_list_split(hdr_copy);
+	curlist = lst;
+
+	/* compile the name */
+	caption = g_strdup("");

same here, use a GString.
+++ composer/e-msg-composer-hdrs.h	2004-01-10 01:20:45.000000000 -0500
@@ -82,7 +82,7 @@
	E_MSG_COMPOSER_VISIBLE_CC         = (1 << 3),
	E_MSG_COMPOSER_VISIBLE_BCC        = (1 << 4),
	E_MSG_COMPOSER_VISIBLE_POSTTO     = (1 << 5),  /* for posting to
folders */
-	E_MSG_COMPOSER_VISIBLE_NEWSGROUP  = (1 << 6),  /* for posting to
newsgroups */
+	/*E_MSG_COMPOSER_VISIBLE_NEWSGROUP  = (1 << 6),*/  /* for posting to
newsgroups */

i take it this was dropped in favour of postto, should remove it not
just comment it.

PS by this stage i'm kinda tired of reviewing this long patch so the
rest i wont look at too closely ...

Sorry. Same goes for me BTW :)

EMFolderSelectionButton *button)
{
	if (response == GTK_RESPONSE_OK) {
-		const char *uri = em_folder_selector_get_selected_uri (emfs);
+		if (button->priv->multiple_select) {
+			GList *uris = em_folder_selector_get_selected_uris (emfs);
+
+			em_folder_selection_button_set_selection_mult (button, uris);
+			g_signal_emit (button, signals[SELECTED], 0);

Do you have to free the uri list here?

No, em_folder_selection_button_set_selection_mult stores it in the struct itself.

+void
+em_folder_selection_button_set_selection_mult
(EMFolderSelectionButton *button, GList *uris)
+{
+	struct _EMFolderSelectionButtonPrivate *priv = button->priv;
+	char *caption, *tmp, *tmp2;
+
+	g_return_if_fail (EM_IS_FOLDER_SELECTION_BUTTON (button));
+
+	if (priv->uris) {
+		g_list_foreach (priv->uris, (GFunc)g_free, NULL);
+		g_list_free(priv->uris);
+		priv->uris = NULL;
+	}
+
+	priv->uris = uris;
+
+	/* compile the name */
+	caption = g_strdup("");
This looks familiar, use a g_string again.

Yeah.


+	while (uris) {
+		tmp = em_utils_folder_name_from_uri(uris->data);
+		if (tmp) {
+			tmp2 = g_strconcat(caption, ", ", tmp, NULL);
+			g_free(caption);
+			caption = tmp2;
+			g_free(tmp);
+			uris = g_list_next(uris);
+		} else {
+			/* apparently, we do not know this folder, so we'll just skip it
*/
+			g_free(uris->data);
+			uris = g_list_next(uris);
+			priv->uris = g_list_remove(priv->uris, uris->data);

priv->uris = g_list_remove_link(priv->uris, uris) is more efficient
(you have to have another pointer to keep next though).
for consistency, use the standard layout:

type
name(args)
{
}
+get_reply_sender (CamelMimeMessage *message, CamelInternetAddress
**to, const char **postto)
{
	const CamelInternetAddress *reply_to;
-	const char *name, *addr;
+	const char *name, *addr, *posthdr;
	int i;
	
+	/* check whether there is a 'Newsgroups: ' header in there */
+	posthdr = camel_medium_get_header (CAMEL_MEDIUM(message),
"Newsgroups");
+	if (posthdr && postto) {
+		*postto = posthdr;
+		while (**postto == ' ') (*postto)++;

Should postto be a list of addresses?  I presume at the moment it is
taken as a space-separated string list of addresses?

A comma-separated list actually. The composer_hdrs_set_post_to splits it up.


+	if (posthdr && postto) {
+		*postto = posthdr;
+		while (**postto == ' ') (*postto)++;

This looks familiar.  Any way to reuse the code?  Should probably have
a camel-mime-utils function to parse the newgroups header into a list
of newsgroups.

We have a CamelNewsAddress class which cam be used like the
CamelInternetAddress 'to' and 'cc' addresses, to store a list of
addresses (newgroups).  Perhaps that should be finished/used
everwhere, rather than strings (but it might need a lot of work yet).

Last time I looked at it, it was pretty much empty :)

/* FIXME: mailto?  url?  should make up its mind what its called.
imho use 'uri' */

Not my fault...

+static void sub_row_activated(GtkTreeView *tree, GtkTreePath *path, GtkTreeViewColumn *col, EMSubscribe *sub) {
+	EMSubscribeNode *node;
+	GtkTreeIter iter;
+	GtkTreeModel *model = gtk_tree_view_get_model(tree);
+
+	if (gtk_tree_model_get_iter(model, &iter, path) != TRUE) return;
add line feeds to the prototype and if statments.






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