Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
On Thu, 2015-10-08 at 10:34 -0500, lib...@acm.org wrote: > I'm certainly open the idea of contributing this feature > to Evolution proper, though. How would we explore this option > further? Hi, simply file an enhancement request against evolution's Composer component at https://bugzilla.gnome.org and attach your patch against git master checkout of the evolution there, then it'll be reviewed and either iterated upon it or just committed into the sources. One common thing for the new contributors, please follow the used coding style, thus other contributors can read the code "easily". Thanks and bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
On Tue, 2015-10-06 at 23:01 -0500, Ben Liblit wrote: > I've tested fetching the send activity from the action bar as > outlined in my earlier message. It works well, feels clean, and > requires no changes to Evolution itself. I'm going to proceed using > this approach. Hi, you are welcome. By the way, how do you plan to "distribute" your plugin/module? Maybe it could be part of the evolution sources, a "core" module, or even better being fully integrated in the composer as a standard functionality. Maybe. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
Milan Crha wrote: > I'm not aware of any such race condition off head. As the composer > andthe related parts are widgets, they operate these changes in the > main (UI) thread, which gives a thread safety. Ah, good point. > I also think it is fine, and the direct get of the activity as you > outlined above is even nicer than checking with notification signal > on the activity bar. I've tested fetching the send activity from the action bar as outlined in my earlier message. It works well, feels clean, and requires no changes to Evolution itself. I'm going to proceed using this approach. I don't see a need to add the "postsend" signal at this time. Milan, thanks again for your creative ideas and suggestions! Regards, Ben ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
On Mon, 2015-10-05 at 20:10 -0500, Ben Liblit wrote: > Is there any possible race condition under which the activity might > complete/cancel and go away before I have an opportunity to grab it > like this? Or is there a race condition under which some *other* > activity might be set as the activity bar's current activity before I > can grab this one? Hi, I'm not aware of any such race condition off head. As the composer and the related parts are widgets, they operate these changes in the main (UI) thread, which gives a thread safety. > I like this! Unless you think there's a race lurking in my code to > fetch the current EActivity, this seems to give me everything I need > with *no* API changes at all. I also think it is fine, and the direct get of the activity as you outlined above is even nicer than checking with notification signal on the activity bar. > I don't quite understand your "double asked" concern. I thought of not copying the e_msg_composer_send() yourself, but just emitting the "presend" signal to check whether the message will be send or not. Forget it, maintaining the copy of that function is not a good option, it's just an option :) > I agree that we should not ask existing callers to g_object_unref() a > return value that they previously did not expect to see at all. If > we believe that ignoring this return value will be the common case, > is it reasonable to return the EActivity *without* increasing its > reference count accordingly? Yes, I'm just used to thread safety issues with references, though it doesn't apply here (see above the note about the main (UI) thread). > That would work for me provided we have some strong guarantee that no > other send can appear in the middle, as in: The send within one composer cannot interleave, because the composer disables most of the UI while processing the send. The solution with added signal is the clean way of achieving what you want. All the above is doable, but a little hacky. If you are fine with the current way, then even better, otherwise I can add the signal and you can use it conditionally (using g_signal_lookup()), or create your plugin only for 3.20+. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
On Sat, 2015-10-03 at 17:21 -0500, Ben Liblit wrote:
> (A good start might be for e_msg_composer_send()
> to return the EActivity it creates: this exactly represents the
> current send attempt and could be monitored for status changes.)
Hi,
you are right, there is no good (and direct) API to achieve what you
want right now. There is an indirect API. The EMsgComposer::busy
property gives a status whether the composer is doing anything. Looking
into the implementation of the busy state change, the code relies on
the EActivityBar notifications, which gets to the EActivity.
Your "Send & Archive" method might look like this (pseudo-code):
static void
action_send_and_archive_cb (GtkAction *action,
EMsgComposer *composer)
{
html_editor = e_msg_composer_get_editor (composer);
activity_bar = e_html_editor_get_activity_bar (editor);
g_signal_connect (
activity_bar, "notify::activity",
G_CALLBACK (composer_sent_and_archive_notify_activity_cb),
composer);
e_msg_composer_send (composer);
}
Where in the composer_sent_and_archive_notify_activity_cb() you'd get
to the activity and you'd disconnect from the "notify::activity"
signal.
It's still not good, because the e_msg_composer_send() can finish
without running any activity, and it can wait for a user confirmation
for an unknown time.
The only current feasible way seems to me the code duplication, copy
the implementation of the e_msg_composer_send() into your plugin (with
other necessary parts) and call that, instead of calling
e_msg_composer_send(). You cannot invoke EMsgComposer::presend signal
before calling e_msg_composer_send() to verify whether the message will
be send or not, because, if any signal listener asks users for
anything, that could be double asked.
I do not think returning an EActivity from e_msg_composer_send() is a
good idea. The reason is that it's an API change. The thing is that the
returned pointer should be referenced, thus each call would need to
unref it. That means adding more responsibilities to the caller and
other overheads.
What about adding a "postsend" signal? The round trip of signals would
be:
presend
send
postsend
where the postsend will be called always after the send is complete,
successfully or failed. That will also mean an API (ABI) change (the
EMsgComposerClass structure will be larger), but it'll not add any
additional requirements for the callers.
What do you think?
Bye,
Milan
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] EMsgComposer API support for "Send and Archive"
Milan Crha wrote: > you are right, there is no good (and direct) API to achieve what you > want right now. There is an indirect API. Thanks for your ideas, Milan. Using "notify::activity" to retrieve the EActivity representing the send attempt is clever, but I agree that it feels somewhat brittle. Perhaps we can do this more directly: e_msg_composer_send(composer); EActivity *activity = e_activity_bar_get_activity( e_html_editor_get_activity_bar( e_msg_composer_get_editor(composer))); if (activity != NULL) g_signal_connect(activity, "notify::state", ...); That ought to give me the activity that e_msg_composer_send() just created (or NULL if it decided not to). Is there any possible race condition under which the activity might complete/cancel and go away before I have an opportunity to grab it like this? Or is there a race condition under which some *other* activity might be set as the activity bar's current activity before I can grab this one? If the above works, then the only question is whether I can reliably use "notify::state" on this EActivity to distinguish successful/failed completion of the send attempt. I think that I can. Successsful completion transitions the activity into the E_ACTIVITY_COMPLETED state in composer_send_completed(), defined in "em-composer-utils.c". Failure does not change the activity's state; composer_send_completed() simply reduces its reference count. But that should be fine for me: I only need to act upon successful completion, and that is easy to detect using "notify::state". I like this! Unless you think there's a race lurking in my code to fetch the current EActivity, this seems to give me everything I need with *no* API changes at all. > The only current feasible way seems to me the code duplication, copy > the implementation of the e_msg_composer_send() into your plugin > (with other necessary parts) and call that, instead of calling > e_msg_composer_send(). You cannot invoke EMsgComposer::presend signal > before calling e_msg_composer_send() to verify whether the message > will be send or not, because, if any signal listener asks users for > anything, that could be double asked. I don't quite understand your "double asked" concern. If I were to make my own private copy of e_msg_composer_send(), my private copy would emit the "presend" signal just as the original does, and would eventually call "e_msg_composer_get_message(..., msg_composer_send_cb, ...)", also just like the original. My copy of e_msg_composer_send() would not be calling the original e_msg_composer_send(), so where does this "double ask" happen? That being said, maintaining my own copy of e_msg_composer_send() is not ideal. I'd also need private copies of msg_composer_send_cb(), async_context_free(), and the AsyncContext struct declaration. e_msg_composer_send() directly or indirectly uses all of these but they are not visible outside the original "e-msg-composer.c" source. > I do not think returning an EActivity from e_msg_composer_send() is a > good idea. The reason is that it's an API change. The thing is that > the returned pointer should be referenced, thus each call would need > to unref it. That means adding more responsibilities to the caller > and other overheads. I agree that we should not ask existing callers to g_object_unref() a return value that they previously did not expect to see at all. If we believe that ignoring this return value will be the common case, is it reasonable to return the EActivity *without* increasing its reference count accordingly? That would mean that a caller could ignore this returned value without creating a leak. Conversely, a caller that does want to keep this value would need to know that it should call g_object_ref() it to keep it alive. Even in my case, I think I would not need to call g_object_ref(), since I would not be keeping my own reference to this EActivity. Rather, I'd just be registering a signal callback on it and after that I no longer need to track it myself. Thinking about documentation and also language bindings, it should be enough to annotate e_msg_composer_send()'s return value as "(transfer none)". Per < https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations>, that marks the returned value as not owned by the recipient. My understanding is that GObject introspection bindings, then, should know that the *caller* needs to increment the reference count rather than assuming that e_msg_composer_send() has already done so. > What about adding a "postsend" signal? The round trip of signals > would be: > presend > send > postsend > where the postsend will be called always after the send is complete, > successfully or failed. That will also mean an API (ABI) change (the > EMsgComposerClass structure will be larger), but it'll not add any > additional requirements for the callers. That would work for me provided we have some strong
