Re: [Evolution-hackers] Proposed fix for bug 311512
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
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
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
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
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
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)? If this is the case, then it is effectively too late to 'trap' the event, because evo will already be processing it. Thus (AFAICS) even if camel is in a 'freeze' for folder_changed events, evo will still be firing on every folder_changed occurence. 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? > That would be a much cleaner way of doing it. > > Jeff Let me know if I am write or wrong on the event stuff, and which direction you would suggest. I'm working elsewhere right at the moment, but would be happy to return to this in due course to improve it further. 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
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
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
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
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
