I am making a note here to take a look at this when I get home from California.   Some notes for myself:

In Pan "1" this was handled by sending a 0 size value to the function acache_expire_to_size

Can we do the same with Pan2 in article-cache.cc expire_to_size ()?
This function accepts no values and has a pointer to the value max_megs.  Since Pan seems to dynamically clear the cache when running I will probably need to copy most of this function into a new one to handle this that is called only when shutdown.

Charles if this all sounds too hackish let me know.

Charles, I am trying to follow the logic in the expire_to_size function and I have a quick question.  What is an "Active" cached article?

If you're referring to the debug message at the end of expire_to_size(),
it's poor wording.  `active' would be better written here as `remaining'.
I agree with you about old Pan's approach.  We could have two functions,
expire_to_size(size_t) and expire_to_size():

  expire_to_size()
  {
    size_t max_bytes = ...;
    expire_to_size (max_bytes);
  }

  expire_to_size(size_t max_bytes)
  {
    quarks_t removed;
    ...
  }

  clear ()
  {
    expire_to_size (0);
  }


The rest of the work is adding a prefs-ui patch for toggling the flag,
and some code in pan.cc right before DataImpl goes out of scope 
to call cache.clear () if the prefs flag is set.

I'll probably be doing a release on Monday that badly breaks the
string freeze.  If you want to do a queue/prefs-ui/pan.cc patch
this weekend, attach it and I'll put it in the release.

That sounds good, I have a tentative patch now that I wrote on the plane but I think something is wrong with my logic since there are always a small number of remaining "Active" objects.  

I plan on diving deeper in it tonight and I hope to have something soon.

I was originally planning on just having the option in preferences.xml but I can easily add it to the preferences options in the GUI since you are breaking the string freeze anyway!

Ok something is odd about the function expire_to_size(), it never catches all the files.

If I do _mid_to_info.size() it returns the proper number of objects in cache, however when I run through the for loop that orders the files it does not pick up every file.  

However if I add a while (_mid_to_info.size()>0) at the top it eventually catches all the files.  So I /think/ something is wrong with the process by which articles are added to the set si of type sorted_info_t but I haven't been able to figure out what yet.

Charles, any ideas?  Am I just doing something stupid?

Steps to reproduce this:
hardcode max_bytes to 0, add a count to the for loop in expire_to_size() to count how many times it runs through.  Then do a _mid_to_info.size() and it is smaller almost everytime.

If you force it to run with a while(_mid_to_info.size()>0) it will eventually catch all the files...

Ahhh, I think I know what's going on.  If two articles have the same
timestamp then they're considered to be equal by MsgInfoCompare.
Great work finding this bug... here's a patch that should fix it.

Index: article-cache.h
===================================================================
--- article-cache.h     (revision 91)
+++ article-cache.h     (working copy)
@@ -98,7 +98,9 @@
 
       struct MsgInfoCompare {
         bool operator()(const MsgInfo& a, const MsgInfo& b) const {
-          return a._date < b._date;
+          if (a._date != b._date)
+            return a._date < b._date;
+          return a._message_id < b._message_id;
         }
       };
 
As an aside, !_mid_to_info.empty() is better than _mid_to_info.size()>0,
since the former only needs to look for a first node, whil the latter
may have to walk the tree to count the nodes.

Created attachment 80771 [details] [review]
patch to create a preference option to clear the article cache on exit

First draft of patch to add an option to clear cache on exit.  

This is pretty simple, I copied the expire_to_size function into a new function called clear_cache_on_exit and call it when they user quits.

This patch doesn't work when someone hits alt-f4 and only when the user exits from the file menu.

Charles, what do you think about a clear cache option in the Articles menu for "on demand" clearing?

Created attachment 80775 [details] [review]
Patch to create an option to clear cache on exit & add a clear article cache option to the article menu

This patch is similar to the one above except it adds a "Clear Article Cache" selection to the articles menu.

Charles:
1) I incorporated your earlier patch into these two.
2) Is the Articles menu the appropriate place for this or would it be better suited for the file menu?

The function shouldn't be called clear_on_exit(), since there's nothing
intrinsic in the function about "on exit".  It could clear at any time.

IMO the approach in Comment #3 would be better, since it would duplicate
less code.

It could work on both exit and alt-F4 if you move the flag test from gui.cc
to pan.cc between the two close braces on lines 297 and 298; that is, right
before the Prefs and Cache objects are destroyed by going out of scope at
the second close brace.

What are the use cases for on-demand clearing?
I'm not really against it but I don't see the need for it either...

I actually changed the name of the function to clear_cache in my second patch since I realized that it didn't make much sense.

Some people have asked how to clear the cache and currently the answer is to go to ~/home/.pan2/article-cache and delete everything which isn't user friendly.  This places it in an easy to find location.  I actually added this after a post today to the users mailing list asking where the cache was.

I think the use case is that a user has increased the size of his cache and wants to do a one-time clear after reading a large number of groups and doesn't want to hassle with:
a) Manually deleting the files, they may not know where it is
b) Setting it to clear on exit and then remember to uncheck that when they open it back up.

Oh and I will fix the exit issue and move it to pan.cc as you pointed out.  Thanks!

Can you clarify what you meant in comment#3?   I /think/ you are suggesting merging the two functions and then just passing the appropriate value to the function depending on what is required.  If so I'll take a look at that.

Sorry for the disjointed replies to this bug report.

Since ArticleCache::clear() is as easy to understand as
ArticleCache::clear_cache(), IMO the function doesn't
need `cache' in the name either... but I'm just nitpicking
at this point. =)

Yes, I'm suggesting that there only be one function that
actually deletes the files, and that it take a size
argument a la 0.14.x.  This should be a private function,
and there should be two public ones, clear() and
expire_to_size(), pass the appropriate value to the
private function expire_to_size(size_t).

No problem about the disjointed replies.

Created attachment 80797 [details] [review]
Try #3 incorporating Charles recommendations

This patch separates the clearing of the cache into two public and one private members, the private function handles all the file deletions as Charles suggested.   

expire_to_size sends the bytes to the private function expire_to_size
clear () sends 0 to the private function expire_to_size

Charles a few notes:
1) This patch includes your fix from comment #7
2) This patch also includes the addition of a clear cache option in the articles menu but if you don't feel it is necessary I can remove it.   However I think it is useful and should probably be present somewhere so users can clear their cache without the need to go outside pan.

Thank you for all the guidance Charles!

Looks great!

er, except main() doesn't take a Prefs&, but still... =)