Re: [Evolution-hackers] initial NNTP patch


But could you re-do the patch using -u3 to diff?  Patch isn't liking it and not applying it automatically.  I'd like to apply it and give it a test!

Also, first impressions, mostly just stylistic issues - read the HACKING file, you're using some things like c++ comments, gchar's etc for the basic types (at least in camel we prefer using the basic c types for basic c types), and not using k&r 'linux kernel' style indenting.


>               CamelFolderInfo *i = (CamelFolderInfo*)g_ptr_array_index(groups, t);
>               if (!i) { return NULL; }
>               if (strcasecmp(i->full_name, name) == 0) return i;

should really be

CamelFolderInfo *info = ...;

if (info == NULL)

if (...)
    return info;

(also, don't use 'i' for other than loop variables, use 'fi' if you want for CamelFolderInfo, as is used in other places in the code).

(actually that whole method heir_find_folder is questionable, should probably use a hashtable, a linear search will get slow with a big list like a newgroup list).

On Tue, 2003-12-09 at 10:16, Meilof wrote:

Just to get some review before I hear the _whole_ patch is messed up, I 
proudly annouce this initial NNTP patch for Evolution:

It does subscribing/unsubscribing and caching of the newsgroup list, and 
it sorts the newsgroup list too. Please let me know whether I'm on the 
right track.

Also, couple of issues:

    * Loading a complete newsgroup list still takes a considerable
      amount of time, largely due to the sorting I have implemented. If

Won't it just take a lot of time anyway?  It could be a long list.  If you have a tree view (like in the subscribe dialogue), then you could list more incrementally (i can't remember, you can do a list of alt.% or something to get a given level without having to get everything?).

Its only real slow the first time isn't it?

      you want to subscribe to the z.z.z newsgroup, this means you have
      to wait quite some time. I propose that if you click on the "z"
      folder of the "z.z.z" newsgroup, it will get updated directly
      rather than after all previous folders, so that if you know what
      you want to find, you can subscribe/unsubscribe easily.
    * I think it would be kind of nice to have short folder names, like
      "a.b.mp3" for alt.binaries.mp3. I did implement this functionality
      in the NNTP store (a configuration setting of the newsgroup), but
      this is obviously something that should be handled by the mailer
      rather than the NNTP backend.

Good idea.  It could go at any level, the backend isn't such a bad spot to put it, how you've done it now, so long as it properly maps to the long names for subscriptions etc.

    * The bounty description mentions the store summary having a "flat"
      and "tree" mode. I just did a s/IMAP/NNTP of the
      imap-store-summary, which seems to work currently, but this is
      probably not what you indended. Can you tell me in concrete where
      these flat and tree modes would be used withing Evo, and what kind
      of functions need to be written to support that?

What i meant by this is either:



for the display mode in the tree view (you could of course collapse them if they have an empty parent).

I guess it would make sense to always do the latter for the subscription dialogue, and have it optional for the frontend, but the api doesn't really cater for alternatives since they both go through the same interface.

    * For offline operation, would it be a good idea to subclass the
I don't think discosummary is needed (i can't remember what its for - maybe needed for posting, but maybe not, news posting is different/easier than imap), but you would need to subclass discostore and maybe discofolder.

I had hoped to work on changing those interfaces but didn't get around to it, so for now they should be used.

I think that was pretty much it. The patch seems to work for me, and 
I'll go over to subscribing to groups first now I think. I have posted 
some screenshots (never mind the desktop environment I was running... :S) on

Send in a unified diff and i'll have a closer look at it!



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