Re: [Evolution-hackers] NNTP cross-posting

[Not Zed <notzed ximian com>]

On Mon, 2004-01-05 at 14:06 +0100, Meilof wrote:
> Not Zed wrote:
> 
> > the logic doesn't actually do that, it just makes sure that if you're 
> > not doing post-to that you always have a 'to' box.  The else part is 
> > actually redundant (it sets a flag it test for in the if).  So does it 
> > really need changing?
> 
> I wasn't completely clear here, but I meant the code in 
> header_set_visibility that _does_ do a check for "visible & 
> h->visible_mask", so it checks whether it is in the mask. This prevented 
> the Post header from popping up in compose.

I dunno it just looked to me that it only enabled it if it was turned
on.

> Thus, in my patch, I uncommented the "& h->visible_mask" part there and 
> that pretty much solved it.

Well something broke.  When you reply to emails it brings up a 'post'
composer with no folder set, and you can't turn off the post entry, and
you have to turn on the to entry manually.  Something isn't quite right
there.

Also, when yuou send mails, they get put in the outbox but aren't sent
immeediately, so somehting else broke there too.

> >>And another thing: how about posting to multiple newsgroups at the same 
> >>time?
> >>
> > I'm  not sure here.  It would be nice to be able to post to multiple 
> > folders, even if you have to select them one at a time.  And things 
> > like 'reply to all' should function as expected too.
> 
> It would be nice indeed, but how should it be done? Just add a menu item 
> with "Add new Post header" or something to the composer? Reply-to-all 
> currently only answers to the current newsgroup (though it does also 
> reply to the sender of the message)...

I'm not sure how that'd work with the post foldre button though.

Pehraps you could remove that and just have an entry there, and have the
'Post:' button bring up a multi-folder-select selector, although that
sounds like a lot of work, and could probably wait.

> > Does NNTP do SASL stuff?
> 
> Hmm.... I don't think so now. It does TLS 
> (http://www.karlsruhe.org/rfc/draft-ietf-nntpext-tls-nntp-01.txt). Don't 
> know anything about how TLS works though...

Ok, no problem.

Anyway I tested the patch, a few issues
 - depending on the order you subscribe to folders, you can get a tree
being displayed in the folder list, including duplicates.

e.g.
 test.a
 test.a.b
 test.a.c

if you subscribe to test.a.b then test.a, i think it does this
(frommemory).

 - the above-mentioned reply button brings up a post composer, even on a
mail folder

 - you should use the CAMEL_FOLDER_NOSELECT flag on non-selectable/
nonsubscribable folders returned from get_folder_info(), rather than
popping up an error box if you try to subscribe to them, the
subscription dialogue will automatically not let you subscribe to them.

 - some sytlistic things in the patch
dont do this:
if () foo;
do this instead:
if ()
  foo;

+static enum { fail, error, success }
+nntp_folder_download_message(CamelStream **retstream, CamelNNTPStore
*nntp_store,
+                             CamelNNTPFolder *nntp_folder, const char
*msgid, CamelException *ex)

i'd prefer this just returned an 'int' and used -1 for failure.

+	if (url->user)
+		fi->url = g_strdup_printf ("nntp://%s %s/%s", url->user, url->host,
si->path);
+	else
+		fi->url = g_strdup_printf ("nntp://%s/%s";, url->host, si->path);

This should probably use camel_url_to_string stuff, otherwise components
may not be properly encoded.

+			fi = nntp_folder_info_from_store_info(store-
>do_short_folder_notation, url, si);
+			fi->flags |= CAMEL_FOLDER_NOINFERIORS | CAMEL_FOLDER_NOCHILDREN;
+			if (!fi) continue;

you check fi after dereferencing it here.

+static CamelFolderInfo *
+nntp_store_get_cached_folder_info(CamelNNTPStore *store, const char
*_top, guint flags, CamelException *ex)
+{


probably prefer to use intop or something rather than _top.

+			camel_object_trigger_event (CAMEL_OBJECT (nntp_store),
"folder_subscribed", fi);

and similar camel-object methods, shouldn't be cast to CAMEL_OBJECT.

Also as a rule we use direct C casts rather than macro casts on internal
methods (although camel's casts are cheapish they're still more
expensive and code-bulky).

		prompt = g_strdup_printf (_("Please enter the NNTP "
+					    "password for %s %s"),
+					  service->url->user,
+					  service->url->host);

generally merge strings like this onto one line, its not that long.

+	/* Check for 'authentication required' codes */
+	if (u == NNTP_AUTH_REQUIRED) {
+		if (camel_nntp_try_authenticate(store)) goto command_begin_send;
+	}
+

do this like:

 if (u == NNTP_AUTH_REQUIRED
     && camel_nntp_try_authenticate(store))
	goto ....;


 EMsgComposer            *e_msg_composer_new
(void);
 EMsgComposer            *e_msg_composer_new_post
(void);
+EMsgComposer            *e_msg_composer_new_mail_post
(void);
 
^^ i wonder if it should just have 1 entry point with a type field.
Each function is almost identical apart from an internal parameter
change.

 static CamelMimeMessage *
-composer_get_message (EMsgComposer *composer, gboolean post, gboolean
save_html_object_data)
+composer_get_message (EMsgComposer *composer, gboolean post, gboolean
save_html_object_data, gboolean *no_recipients)
 {
 	CamelMimeMessage *message = NULL;
 	EABDestination **recipients, **recipients_bcc;
@@ -296,10 +296,14 @@
 	camel_object_unref (cia);
 	
 	/* I'm sensing a lack of love, er, I mean recipients. */
-	if (num == 0 && !post) {
-		e_notice ((GtkWindow *) composer, GTK_MESSAGE_WARNING,
-			  _("You must specify recipients in order to send this message."));
-		goto finished;
+	if (num == 0) {


Are you sure this change is right?  How could you ever setup a composer
with no post or no to set?  One should be set?

 Michael