> --- /home/meilof/tmp/evolution/camel/providers/nntp//camel-nntp-folder.c
> 2003-07-09 21:21:58.000000000 +0200
> +++ camel-nntp-folder.c 2003-12-09 17:34:43.000000000 +0100
> @@ -69,8 +69,9 @@
>
> CAMEL_NNTP_STORE_LOCK(nntp_store, command_lock);
>
> - if (camel_nntp_summary_check((CamelNNTPSummary *)folder->summary,
> nntp_folder->changes, ex) != -1)
> + if (camel_nntp_summary_check((CamelNNTPSummary *)folder->summary,
> nntp_folder->changes, ex) != -1) {
> camel_folder_summary_save (folder->summary);
> + }
no need for the braces here. the rule is that if there is only 1 line,
don't use braces unless the 'else' section is more than 1 line.
ie:
if (foo)
bar ();
if (foo) {
bar ();
} else {
baz ();
bong ();
}
>
> if (camel_folder_change_info_changed(nntp_folder->changes)) {
> changes = nntp_folder->changes;
> @@ -130,10 +131,12 @@
> if (ret == 220) {
> stream = camel_data_cache_add(nntp_store->cache, "cache",
> msgid, NULL);
> if (stream) {
> - if (camel_stream_write_to_stream((CamelStream
> *)nntp_store->stream, stream) == -1)
> + if (camel_stream_write_to_stream((CamelStream
> *)nntp_store->stream, stream) == -1) {
> goto error;
> - if (camel_stream_reset(stream) == -1)
> + }
> + if (camel_stream_reset(stream) == -1) {
> goto error;
> + }
> } else {
> stream = (CamelStream *)nntp_store->stream;
> camel_object_ref((CamelObject *)stream);
same here and all other places.
> @@ -274,7 +277,7 @@
> {
> struct _CamelNNTPFolderPrivate *p;
>
> - g_free(nntp_folder->storage_path);
> + camel_folder_summary_save(nntp_folder->parent.summary);
to be consistant with the rest of the code, you should do:
camel_folder_summary_save (((CamelFolder *) nntp_folder)->summary);
>
> --- /home/meilof/tmp/evolution/camel/providers/nntp//camel-nntp-store.c
> 2003-09-22 17:00:59.000000000 +0200
> +++ camel-nntp-store.c 2003-12-09 17:33:50.000000000 +0100
> @@ -42,6 +42,7 @@
>
> #include "camel-nntp-summary.h"
> #include "camel-nntp-store.h"
> +#include "camel-nntp-store-summary.h"
> #include "camel-nntp-folder.h"
> #include "camel-nntp-private.h"
>
> @@ -65,6 +66,17 @@
> #define CF_CLASS(so) CAMEL_FOLDER_CLASS (CAMEL_OBJECT_GET_CLASS(so))
> #define CNNTPF_CLASS(so) CAMEL_NNTP_FOLDER_CLASS (CAMEL_OBJECT_GET_CLASS(so))
>
> +static void nntp_construct (CamelService *service, CamelSession *session,
> + CamelProvider *provider, CamelURL *url,
> + CamelException *ex);
> +
> +void nntp_store_sumaryinfo_to_grouptree(CamelNNTPStore *nntp_store,
> + GPtrArray *groups,
> +// CamelFolderInfo **groups, CamelFolderInfo
> **last,
> + const char *name, const char *top, int
> flags,
> + CamelStoreInfo *si, CamelURL *url);
s/sumary/summary/
also, don't use c++ style comments (I realise c99 allows them, but we prefer (and
still have to compile under) c89)
> +
> +CamelFolderInfo *nntp_folder_info_from_store_info(gboolean shortn, CamelURL *url,
> CamelStoreInfo *si);
>
> enum {
> USE_SSL_NEVER,
> @@ -88,7 +100,6 @@
> /* setup store-wide cache */
> if (store->cache == NULL) {
> char *root;
> -
> root = camel_session_get_storage_path (service->session, service, ex);
> if (root == NULL)
> goto fail;
> @@ -155,7 +166,7 @@
> goto fail;
> }
>
> - len = strtoul (buf, (char **) &buf, 10);
> + len = strtoul (buf, (char **)&buf, 10);
don't gratuitously change whitespace :-)
> if (len != 200 && len != 201) {
> camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
> _("NNTP server %s returned error code %d: %s"),
> @@ -294,17 +305,222 @@
> return folder;
> }
>
> -static CamelFolderInfo *
> -nntp_store_get_folder_info(CamelStore *store, const char *top, guint32 flags,
> CamelException *ex)
> -{
> +gchar *camel_nntp_store_get_toplevel_dir(CamelStore *store, CamelException *ex) {
> + return camel_session_get_storage_path (CAMEL_SERVICE(store)->session,
> CAMEL_SERVICE(store), ex);
> +}
don't use g<type>'s, it messes up syntax highlighting and all that. we
prefer c-types for camel (and mail too)
also, functions are done like:
<return-type>
function_name (arg1, arg2, ...)
{
<statements>
}
> +
> +/*
> + * Converts a fully-fledged newsgroup name to a name in short dotted notation,
> + * e.g. nl.comp.os.linux.programmeren becomes n.c.o.l.programmeren
> + */
> +
> +gchar *nntp_newsgroup_name_short(char *name) {
> + gchar *tmp = g_malloc0(strlen(name) + 1), *resptr = tmp, *ptr2;
always place a blank line after the end of variable declarations. (and use char
instead of gchar)
> + while ((ptr2 = strchr(name, '.'))) {
> + if (ptr2 == name) { name++; continue; }
don't do this.
do this instead:
if (ptr2 == name) {
name++;
continue;
}
> + *resptr++ = *name;
> + *resptr++ = '.';
> + name = ptr2 + 1;
> + }
adding a blank line after loops makes the code more readable.
> + strcpy(resptr, name);
> + ptr2 = g_strdup(tmp);
> + g_free(tmp);
> + return ptr2;
> +}
what's the point of strdup'ing tmp? tmp is already a malloc'd buffer
containing the exact value you want to return.
> +
> +/*
> + * This function converts a NntpStoreSummary item to a FolderInfo item that
> + * can be returned by the get_folders() call to the store. Both structs have
> + * essentially the same fields.
> + */
> +
> +CamelFolderInfo *nntp_folder_info_from_store_info(gboolean short_notation,
> + CamelURL *url, CamelStoreInfo *si) {
> + CamelFolderInfo *fi = g_malloc0(sizeof(*fi));
blank line here. also the return-type should be on its own line.
> + if (short_notation)
> + fi->name = nntp_newsgroup_name_short(si->path);
> + else
> + fi->name = g_strdup(si->path);
> + fi->full_name = g_strdup(si->path);
> + fi->url = g_strdup(si->uri);
> + fi->unread_message_count = -1;
> + // TODO: why '/' rather than '.'?
> + camel_folder_info_build_path(fi, '/');
fi->path is the 'canonicalised' path used by the UI (folder-tree). Not
as important these days, but folders used to get added to the tree based
on its path rather than the structure of the CamelFolderInfo's.
must be delimited by '/' which also means that if the user doesn't want
a flat list of newsgroups, you'll have to replace '.' with '/' for
full_name too.
> +
> + /* set URL */
> + if (fi->url) return fi;
if (fi->url)
return fi;
> + if (url->user)
> + fi->url = g_strdup_printf ("nntp://[EMAIL PROTECTED]/%s", url->user,
> url->host, si->path);
> + else
> + fi->url = g_strdup_printf ("nntp://%s/%s";, url->host, si->path);
> + return fi;
> +}
> +
> +CamelStoreInfo *nntp_store_info_from_line(CamelStoreSummary *summ, CamelURL *url,
> char *line) {
> + CamelStoreInfo *si = camel_store_summary_info_new(summ);
> + if (url->user)
> + si->uri = g_strdup_printf ("nntp://[EMAIL PROTECTED]/%s", url->user,
> url->host, line);
> + else
> + si->uri = g_strdup_printf ("nntp://%s/%s";, url->host, line);
> + si->path = g_strdup(line);
> + return (CamelStoreInfo*)si;
> +}
not sure this is right or not...
> +
> +/* ----- Get information on subscribed folders ----- */
> +
> +static GPtrArray*
> +nntp_store_get_subscribed_folder_info(CamelNNTPStore *store, const char *top, guint
> flags, CamelException *ex) {
> + if (top == NULL)
> + top = "";
> + int toplen = strlen(top);
variable declarations cannot happen anywhere in a block. they MUST be at
the top. c89 and all that...
> +
> + int i;
> + CamelStoreInfo *si;
> + GPtrArray *groups = g_ptr_array_new();
why isn't this declaration indented the same as the others?
> CamelURL *url = CAMEL_SERVICE (store)->url;
> - CamelNNTPStore *nntp_store = (CamelNNTPStore *)store;
> - CamelFolderInfo *groups = NULL, *last = NULL, *fi;
> - unsigned int len;
> +
> + for (i=0;(si = camel_store_summary_index((CamelStoreSummary
> *)store->summary, i));i++) {
> + // TODO: why doesn't si->path always work for checks?
> + if (strncasecmp(si->path, top, toplen) == 0 &&
> si->flags&CAMEL_STORE_INFO_FOLDER_SUBSCRIBED) {
> + nntp_store_sumaryinfo_to_grouptree(store, groups,
> si->path,
> + top,
> flags|CAMEL_STORE_FOLDER_INFO_RECURSIVE, si, url);
> + }
the indenting is all fubar'd here.
> + camel_store_summary_info_free((CamelStoreSummary *)store->summary, si);
> + return groups;
> +
> +}
> +
> +/* ----- non-subscribed folder list mode ----- */
> +
> +CamelFolderInfo *hier_find_folder(GPtrArray *groups, char *name) {
is this supposed to be a public function? if so, namespace it. 'name' should probably
also be const.
> + int t;
> + for (t = 0; t < groups->len; t++) {
> + CamelFolderInfo *i = (CamelFolderInfo*)g_ptr_array_index(groups, t);
> + if (!i) { return NULL; }
if (!i)
return NULL;
> + if (strcasecmp(i->full_name, name) == 0) return i;
> + }
> + return NULL;
> +// for (int t = 0; t < g_ptr_array_groups
> +// while (groups) {
> +// if (strcasecmp(groups->full_name, name) == 0) { return groups; }
> +// groups = groups->sibling;
> +// }
> +// return 0;
> +}
again with the c++ comments...
> +
> +// TODO: the line argument is probably not nessecary since it's also stored in si
> +void nntp_store_sumaryinfo_to_grouptree(CamelNNTPStore *nntp_store,
> + GPtrArray* groups,
> +// CamelFolderInfo **groups, CamelFolderInfo
> **last,
> + const char *name, const char *top, int
> flags,
> + CamelStoreInfo *si, CamelURL *url) {
> + int toplen = strlen(top);
> + CamelFolderInfo *fi = NULL;
blank line here.
> + /* it is a child item */
> + if (flags & CAMEL_STORE_FOLDER_INFO_RECURSIVE || /* recursive listing */
> + name[toplen] == 0 || /* the item itself */
> + strchr(name + toplen + 1, '.') == NULL) { /* direct subitem */
> + // TODO: if a dummy already exists, use it instead
> + fi =
> nntp_folder_info_from_store_info(nntp_store->do_short_folder_notation, url, si);
> + } else if (!(flags & CAMEL_STORE_FOLDER_INFO_RECURSIVE)) {
> + /* the recursion flag has not been set. what we want to do now is
> + add a one-level parent group, if it does not already exist */
> + gchar *name2 = g_strdup(name);
add a blank line here, makes the code more readable.
> + /* we already know that apparently there is still a dot in there, so
> + we can safely remove it */
> + CamelFolderInfo *folder;
> + *(strchr(name2 + toplen + 1, '.')) = '\0';
> + folder = hier_find_folder(groups, name2);
> + if (!folder) {
> + gchar *line2 = g_strdup_printf("%s 0 0", name2);
> + line2[strlen(name2)] = '\0';
> + si =
> nntp_store_info_from_line(CAMEL_STORE_SUMMARY(nntp_store->summary), url, line2);
> + fi =
> nntp_folder_info_from_store_info(nntp_store->do_short_folder_notation, url, si);
> +
> CAMEL_STORE_SUMMARY_CLASS(CAMEL_OBJECT_GET_CLASS(nntp_store->summary))->store_info_free((CamelStoreSummary*)nntp_store->summary,
> si);
> + g_free(line2);
> + }
> + g_free(name2);
> + }
what's with the crazy indenting here?
> + if (fi) {
> + g_ptr_array_add (groups, (gpointer) fi);
> + }
no need for the braces.
> +}
> +
> +static gboolean nntp_get_date(CamelNNTPStore *nntp_store) {
> + unsigned char *line;
> + int ret = camel_nntp_command(nntp_store, (char **)&line, "date");
> + nntp_store->summary->last_newslist[0] = 0;
> + if (ret == 111) {
> + char *ptr = line + 3; while (*ptr == ' ' || *ptr == '\t')ptr++;
> + if (strlen(ptr) == NNTP_DATE_SIZE) {
> + memcpy(nntp_store->summary->last_newslist, ptr,
> NNTP_DATE_SIZE);
> + return TRUE;
> + }
> + }
> + return FALSE;
> +}
please follow the following convention:
static gboolean
nntp_get_date (CamelNNTPStore *nntp_store)
{
...;
}
> +
> +GPtrArray *
> +nntp_store_get_folder_info_all(CamelNNTPStore *nntp_store, const char *top, guint32
> flags, CamelException *ex)
if this is meant to be public, properly namespace it or else make it static.
> +{
> + CamelURL *url = CAMEL_SERVICE (nntp_store)->url;
> + CamelNntpStoreSummary *summary = nntp_store->summary;
> + CamelStoreInfo *si;
> + GPtrArray *groups = g_ptr_array_new();
> + unsigned int len, i;
> unsigned char *line, *space;
> int ret = -1;
> + if (top == NULL) top = "";
> + int toplen = strlen(top);
you can't do this. c89!!!!!
I'll take a look again when you clean up these issues.
Jeff
_______________________________________________
evolution-hackers maillist - [EMAIL PROTECTED]
http://lists.ximian.com/mailman/listinfo/evolution-hackers
