GNOME Bugzilla – Bug 340783
publishing does not show errors nor success
Last modified: 2013-09-10 14:04:48 UTC
Patch that got lost on the list.
Created attachment 64888 [details] [review] add EError to publishing plugin
It was not lost, i am looking into. It introduces some string changes it can be taken only for 2.8. I will review it asap.
The e_error_run will not display any error message, as the tag passed is not in the form domain:error-id. The ":" is missing in all the e_error_run functions. Calling e_error in a thread causes a, Xlib: unexpected async reply (sequence 0xb1e4)!. Probably e_error_new could be used. e_cal_get_freebusy will fail for exchange and groupwise calendars as the list of users is not passed. The cal_address of the selected calendar should be passed in the user_list. The email address of the default account could be used when the cal_address is not available. The errors when the functions fails must be handled. The error message "Couldn't create the uri {0}" can be "Could not the uri {0}". It lacks a secondary error message. In the function publish () [publish-calendar.c:82], the null check for the vfs_uri should be present after gnome_vfs_uri_new (). Please include the ChangeLog with the patch.
*** Bug 261836 has been marked as a duplicate of this bug. ***
I'm not sure what you mean by: The error message "Couldn't create the uri {0}" can be "Could not the uri {0}". Can you clarify?
Created attachment 67680 [details] [review] patch This fixes all your comments except for the contents of the uri-creation error
I guess Chen meant a) s/Couldn't/Could not/ and b) It requires a secondary error message.
chen: *poke*! can we please get this reviewed within the next two weeks?
Created attachment 68631 [details] [review] new version New version with error.xml problems fixed.
This patch will not popup a error message as the widget returned by e_error_new is not shown at all. The contents of the groupwise calendar is getting published now. Another issue which i found while reviewing is in publish-calendar.c, function publish ():84 if (vfs_uri == NULL) { } The password variable should not be freed in this block. Was this patch tested ?
david: ping
Created attachment 69963 [details] [review] 20060731 Fixes the list of issues
hmm. punting, this cannot go into 2.8 anymore as it would break string and UI freeze now. anyway, chen, a patch review and a "commit-after-freeze" status would be of course welcome. :-)
2.9 target. definitely.
Xlib: unexpected async reply (sequence 0x6975)! Am still getting the hang. The error dialog comes up properly for the first time. Evolution hangs when it comes for the second time with the above message on the console.
Hmm. This sounds like a thread problem. I'll update the patch to HEAD and try to reproduce.
any news here?
Bumping version to a stable release.
*** Bug 392291 has been marked as a duplicate of this bug. ***
Requested to show also a success of an operation.
Created attachment 130076 [details] [review] proposed evo patch for evolution; Show success only when explicitly called by menu Actions->Publish..., otherwise shows only error messages, if any.
The patch looks good. Just one small concern, Will the UI hang if the error_queue_show_idle thread gets locked ? If thats not the case the patch can be committed after the freeze.
(In reply to comment #22) > Will the UI hang if the error_queue_show_idle thread gets locked ? It's locked for time of extracting messages from a queue only, but the lock is released (g_static_mutex_unlock (&error_queue_lock);) before the extracted messages are shown to a user (e_notice call), thus should be fine. I'm thinking of some improvements of this approach, moving the rutines to e-util or somewhere, and let each component to register for listening to these queues, with some distinguishment on "domain" basis, so some messages could be shown in a status bar only, not bothering user with a dialog. Though that's a question of the next step, improvement, not this particular bug. (I'm just thinking aloud.)
chen, ping
Ok it shouldn be a problem. Please commit the patch to trunk as its branched now.
Created commit f171b15 in master.