Re: [Evolution-hackers] Proposed fix for bug 311512

2007-06-01 Thread Karl Relton
Review from anyone? ...



Hi friends

I have submitted new patches to bugzilla for bug

http://bugzilla.gnome.org/show_bug.cgi?id=311512


One changes e-d-s as Fejj suggested, the other does the Evolution side
so it will only do mail-notification on 'truely' new messages.

I'll be pleased if you could review.

Karl


On Thu, 2007-04-26 at 09:29 -0400, Jeffrey Stedfast wrote:
 not completely correct... 
 
 when you trigger an event on a CamelObject, it first fires the prep
 callback, which is what camel-folder.c:folder_changed() is (note that it
 returns bool)
 
 A prep event handler is the first handler called (event handlers are
 fired sequentially, in order of connection - /not/ in parallel) and gets
 to decide if the event propagates by returning TRUE (or FALSE if it
 should be blocked - that's how freeze/thaw works).
 


___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-05-20 Thread Karl Relton
Hi friends

I have submitted new patches to bugzilla for bug

http://bugzilla.gnome.org/show_bug.cgi?id=311512


One changes e-d-s as Fejj suggested, the other does the Evolution side
so it will only do mail-notification on 'truely' new messages.

I'll be pleased if you could review.

Karl


On Thu, 2007-04-26 at 09:29 -0400, Jeffrey Stedfast wrote:
 not completely correct... 
 
 when you trigger an event on a CamelObject, it first fires the prep
 callback, which is what camel-folder.c:folder_changed() is (note that it
 returns bool)
 
 A prep event handler is the first handler called (event handlers are
 fired sequentially, in order of connection - /not/ in parallel) and gets
 to decide if the event propagates by returning TRUE (or FALSE if it
 should be blocked - that's how freeze/thaw works).
 


___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-05-20 Thread Srinivasa Ragavan
Karl,

This is a killer problem and you have been rocking with your patches on
this right from the beginning. Awesome work!

I'm sure that the Mailer hackers would review it asap and It will be
great if we can get this in for 2.11.3.

Thanks for your great work!

-Srini.

On Sun, 2007-05-20 at 19:29 +0100, Karl Relton wrote:
 Hi friends
 
 I have submitted new patches to bugzilla for bug
 
 http://bugzilla.gnome.org/show_bug.cgi?id=311512
 
 
 One changes e-d-s as Fejj suggested, the other does the Evolution side
 so it will only do mail-notification on 'truely' new messages.
 
 I'll be pleased if you could review.
 
 Karl
 
 
 On Thu, 2007-04-26 at 09:29 -0400, Jeffrey Stedfast wrote:
  not completely correct... 
  
  when you trigger an event on a CamelObject, it first fires the prep
  callback, which is what camel-folder.c:folder_changed() is (note that it
  returns bool)
  
  A prep event handler is the first handler called (event handlers are
  fired sequentially, in order of connection - /not/ in parallel) and gets
  to decide if the event propagates by returning TRUE (or FALSE if it
  should be blocked - that's how freeze/thaw works).
  
 
 

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-04-26 Thread Jeffrey Stedfast
On Thu, 2007-04-26 at 07:50 +0100, Karl Relton wrote:
 On Wed, 2007-04-25 at 09:48 -0400, Jeffrey Stedfast wrote:
  I'm not sure I'd call it get_filter_thread() because it's not getting a
  thread, all it is really doing is getting you a 'wait' id (and ugh, the
  new session_thread_wait() just busy-waits?)
  
  I think if this type of solution is really the best way of doing it,
  then I think it'd be better to just have a CamelFolder function
  (camel_folder_wait_filtering()? I dunno)that waits for the filtering
  operation to finish rather than providing a higher level with a wait id
  that it should never have to know or care about.
  
 
 Agreed - if this is the 'best way', I'll redo the patch as you suggest.
 
  
  But better still would be to trap the folder_changed signal for
  folders that are currently in the process of filtering (CamelFolder
  already listens for this event in order to invoke the filtering iirc, so
  just stop the signal from propagating) and re-emit it later when
  filtering is complete (obviously with the updated changes).
  
 
 Yes - I had thought of this too.
 
 The trouble is with the current code its not quite so simple AFAIK. The
 filtering that takes place is actually launched from the
 folder_changed() function in camel-folder.c. In other words, it is
 launched from the folder_changed event handler itself. Now I may be
 wrong here, but my assumption is that both evo and e-d-s register event
 handlers on this type of event - so that when such an event occurs code
 in both evo and e-d-s is executed ... perhaps even in parallel (in their
 own threads)?

not completely correct... 

when you trigger an event on a CamelObject, it first fires the prep
callback, which is what camel-folder.c:folder_changed() is (note that it
returns bool)

A prep event handler is the first handler called (event handlers are
fired sequentially, in order of connection - /not/ in parallel) and gets
to decide if the event propagates by returning TRUE (or FALSE if it
should be blocked - that's how freeze/thaw works).

 
 If this is the case, then it is effectively too late to 'trap' the
 event, because evo will already be processing it.

all the event handler has to do is return FALSE to block other event
handlers from firing :)

  Thus (AFAICS) even if
 camel is in a 'freeze' for folder_changed events, evo will still be
 firing on every folder_changed occurence.

only if folder_changed() returns TRUE - folder_changed() is what checks
if the folder is in a freeze state, and if it is blocks further events
from firing (by returning FALSE).

You have da powah!

 
 One way to solve that would be to change things so that evo only fires
 when camel folder_changed stuff has really done: effectively at the end
 of a freeze AND filtering. That could be done by introducing a new event
 and making evo trigger off that perhaps?

unnecessary. all the tools are already available :)

 
 
  That would be a much cleaner way of doing it.
  
  Jeff
 

Jeff

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-04-26 Thread Karl Relton
On Thu, 2007-04-26 at 09:29 -0400, Jeffrey Stedfast wrote:
  Yes - I had thought of this too.
  
  The trouble is with the current code its not quite so simple AFAIK. The
  filtering that takes place is actually launched from the
  folder_changed() function in camel-folder.c. In other words, it is
  launched from the folder_changed event handler itself. Now I may be
  wrong here, but my assumption is that both evo and e-d-s register event
  handlers on this type of event - so that when such an event occurs code
  in both evo and e-d-s is executed ... perhaps even in parallel (in their
  own threads)?
 
 not completely correct... 
 
 when you trigger an event on a CamelObject, it first fires the prep
 callback, which is what camel-folder.c:folder_changed() is (note that it
 returns bool)
 
 A prep event handler is the first handler called (event handlers are
 fired sequentially, in order of connection - /not/ in parallel) and gets
 to decide if the event propagates by returning TRUE (or FALSE if it
 should be blocked - that's how freeze/thaw works).
 
  
  If this is the case, then it is effectively too late to 'trap' the
  event, because evo will already be processing it.
 
 all the event handler has to do is return FALSE to block other event
 handlers from firing :)
 
   Thus (AFAICS) even if
  camel is in a 'freeze' for folder_changed events, evo will still be
  firing on every folder_changed occurence.
 
 only if folder_changed() returns TRUE - folder_changed() is what checks
 if the folder is in a freeze state, and if it is blocks further events
 from firing (by returning FALSE).
 
 You have da powah!
 
  
  One way to solve that would be to change things so that evo only fires
  when camel folder_changed stuff has really done: effectively at the end
  of a freeze AND filtering. That could be done by introducing a new event
  and making evo trigger off that perhaps?
 
 unnecessary. all the tools are already available :)
 

Excellent - many thanks for correcting me on my understanding. I'll look
into it more to see how this can be worked up. Give me a couple of
weeks, and I'll come back if I find I need more understanding.

Karl

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-04-25 Thread Karl Relton
Hi friends,

Could anyone take a look at my proposed patches
for fixing bug 311512. Srini has had a look, and noted the ABI breakage
(see below). This was deliberate on my part - the only way to get
newmail notification to take note of the results of a filter thread.

Many thanks
Karl

On Fri, 2007-03-30 at 12:43 +0530, Srinivasa Ragavan wrote:
 On Fri, 2007-03-30 at 07:51 +0100, Karl Relton wrote:
  Last week I posted two patches (one for eds, one for evo) on evo bugzill
  that I believe fix bug 311512.
 
  --- camel-folder.h.110  2007-03-20 16:57:40.0 +
  +++ camel-folder.h  2007-03-20 16:50:34.0 +
  @@ -195,6 +195,7 @@ typedef struct {
  void (*freeze)(CamelFolder *folder);
  void (*thaw)  (CamelFolder *folder);
  gboolean (*is_frozen) (CamelFolder *folder);
  +   int  (*get_filter_thread) (CamelFolder *folder);
   } CamelFolderClass;
 
 On first look, I noticed that your patch has introduced an ABI break in
 CamelFolderClass. 
 
 I'm sure that the mailer hackers would have a more closer look at it. 
 
 Thanks for your friendly poke :-) 
 
 -Srini.
 
  f
  Could you take a look - any comments are welcome!
  


___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-04-25 Thread Jeffrey Stedfast
I'm not sure I'd call it get_filter_thread() because it's not getting a
thread, all it is really doing is getting you a 'wait' id (and ugh, the
new session_thread_wait() just busy-waits?)

I think if this type of solution is really the best way of doing it,
then I think it'd be better to just have a CamelFolder function
(camel_folder_wait_filtering()? I dunno)that waits for the filtering
operation to finish rather than providing a higher level with a wait id
that it should never have to know or care about.


But better still would be to trap the folder_changed signal for
folders that are currently in the process of filtering (CamelFolder
already listens for this event in order to invoke the filtering iirc, so
just stop the signal from propagating) and re-emit it later when
filtering is complete (obviously with the updated changes).

That would be a much cleaner way of doing it.

Jeff


On Wed, 2007-04-25 at 08:13 +0100, Karl Relton wrote:
 Hi friends,
 
 Could anyone take a look at my proposed patches
 for fixing bug 311512. Srini has had a look, and noted the ABI breakage
 (see below). This was deliberate on my part - the only way to get
 newmail notification to take note of the results of a filter thread.
 
 Many thanks
 Karl
 
 On Fri, 2007-03-30 at 12:43 +0530, Srinivasa Ragavan wrote:
  On Fri, 2007-03-30 at 07:51 +0100, Karl Relton wrote:
   Last week I posted two patches (one for eds, one for evo) on evo bugzill
   that I believe fix bug 311512.
  
   --- camel-folder.h.1102007-03-20 16:57:40.0 +
   +++ camel-folder.h2007-03-20 16:50:34.0 +
   @@ -195,6 +195,7 @@ typedef struct {
 void (*freeze)(CamelFolder *folder);
 void (*thaw)  (CamelFolder *folder);
 gboolean (*is_frozen) (CamelFolder *folder);
   + int  (*get_filter_thread) (CamelFolder *folder);
} CamelFolderClass;
  
  On first look, I noticed that your patch has introduced an ABI break in
  CamelFolderClass. 
  
  I'm sure that the mailer hackers would have a more closer look at it. 
  
  Thanks for your friendly poke :-) 
  
  -Srini.
  
   f
   Could you take a look - any comments are welcome!
   
 
 

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-04-09 Thread Karl Relton
Matthew

As a new Mail maintainer, could you take a look at my proposed patches
for fixing bug 311512. Srini has had a look, and noted the ABI breakage
(see below). This was deliberate on my part - the only way to get
newmail notification to take note of the results of a filter thread.

Many thanks
Karl

On Fri, 2007-03-30 at 12:43 +0530, Srinivasa Ragavan wrote:
 On Fri, 2007-03-30 at 07:51 +0100, Karl Relton wrote:
  Last week I posted two patches (one for eds, one for evo) on evo bugzill
  that I believe fix bug 311512.
 
  --- camel-folder.h.110  2007-03-20 16:57:40.0 +
  +++ camel-folder.h  2007-03-20 16:50:34.0 +
  @@ -195,6 +195,7 @@ typedef struct {
  void (*freeze)(CamelFolder *folder);
  void (*thaw)  (CamelFolder *folder);
  gboolean (*is_frozen) (CamelFolder *folder);
  +   int  (*get_filter_thread) (CamelFolder *folder);
   } CamelFolderClass;
 
 On first look, I noticed that your patch has introduced an ABI break in
 CamelFolderClass. 
 
 I'm sure that the mailer hackers would have a more closer look at it. 
 
 Thanks for your friendly poke :-) 
 
 -Srini.
 
  f
  Could you take a look - any comments are welcome!
  


___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Proposed fix for bug 311512

2007-03-30 Thread Srinivasa Ragavan
On Fri, 2007-03-30 at 07:51 +0100, Karl Relton wrote:
 Srini
 
 Welcome to your new role (again!).
 
 Last week I posted two patches (one for eds, one for evo) on evo bugzill
 that I believe fix bug 311512.

 --- camel-folder.h.1102007-03-20 16:57:40.0 +
 +++ camel-folder.h2007-03-20 16:50:34.0 +
 @@ -195,6 +195,7 @@ typedef struct {
   void (*freeze)(CamelFolder *folder);
   void (*thaw)  (CamelFolder *folder);
   gboolean (*is_frozen) (CamelFolder *folder);
 + int  (*get_filter_thread) (CamelFolder *folder);
  } CamelFolderClass;

On first look, I noticed that your patch has introduced an ABI break in
CamelFolderClass. 

I'm sure that the mailer hackers would have a more closer look at it. 

Thanks for your friendly poke :-) 

-Srini.

 f
 Could you take a look - any comments are welcome!
 
 Regards
 Karl
 
 ___
 Evolution-hackers mailing list
 Evolution-hackers@gnome.org
 http://mail.gnome.org/mailman/listinfo/evolution-hackers

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


[Evolution-hackers] Proposed fix for bug 311512

2007-03-29 Thread Karl Relton
Srini

Welcome to your new role (again!).

Last week I posted two patches (one for eds, one for evo) on evo bugzill
that I believe fix bug 311512.

Could you take a look - any comments are welcome!

Regards
Karl

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers