GNOME Bugzilla – Bug 762840
Time synchronisation problem in the drop down list
Last modified: 2017-11-02 08:21:50 UTC
Created attachment 322608 [details] Image See attached screenshot: The time interval in the drop down list does not correspond to the latest log message.
Thanks for the report. I can confirm it. To reproduce this bug, open Logs and wait a few minutes(one minute should be fine) for new log entries. Then click "All" category, latest log entries will show up in the list view. The time of the drop down menu is only set when Logs starts. Probably we should update the time every time we click on a category.
Created attachment 323550 [details] [review] Fixes the time synchronization issue when changing category Patch for fixing the time synchronization issue.
Hi I have created a patch for the given issue. Please review it.
Review of attachment 323550 [details] [review]: You should not add an action to GlWindow to monitor the category list change, as you can instead connect to the "category" property change (in other words, the "notify::category" signal) on the GlCategoryList. This should simplify the remaining code a fair bit.
Created attachment 324680 [details] [review] Update time after changing category Added Support for updating the time when a different category is selected.
Review of attachment 324680 [details] [review]: ::: src/gl-journal.c @@ +74,3 @@ + boot_first = &g_array_index (boot_ids, GlJournalBootID, boot_ids->len-1); + + boot_first->realtime_last = g_get_real_time(); The current time has very little to do with the most recent log message, so this does not seem correct.
Review of attachment 324680 [details] [review]: We should also update the time label of the drop boot selection menu. We are going to implement a new feature in bug 753471. After finishing it we can just use "** - now" to show the time label. So is it really necessary to fix this bug? If we fix it ATM, we'll have to drop this patch later on. Anyway, review is down below. ::: src/gl-eventview.c @@ +78,3 @@ + priv = gl_event_view_get_instance_private (view); + + return (priv->events); brackets are not needed here. ::: src/gl-eventviewlist.c @@ +593,3 @@ + priv = gl_event_view_list_get_instance_private (view); + +} Brackets are not needed here as mentioned above. ::: src/gl-journal.c @@ +73,3 @@ + boot_first = &g_array_index (boot_ids, GlJournalBootID, boot_ids->len-1); + You should also add a "if" condition to make sure only change the boot_first->realtime_last while trying to fetch the time of the current boot. ::: src/gl-window.c @@ +260,3 @@ +static const gchar * +get_current_boot_match (GlWindow *window) Is it necessary to add this function? It's only called once in on_category_list_row_change() and this function just simply call gl_event_view_get_current_boot_match. @@ +298,3 @@ + if (g_strcmp0 (boot_match, first_boot_match) == 0) + { + GlEventToolbar *toolbar; Incorrect indentation. @@ +491,3 @@ + categories = GL_CATEGORY_LIST (gl_event_view_list_get_category_list(eventviewlist)); + + g_signal_connect (categories, "notify::category", trailing whitespace.
Created attachment 325238 [details] [review] Update time after changing category Added support for updating the time when a different category is selected.
Review of attachment 325238 [details] [review]: ::: src/gl-journal.c @@ +138,3 @@ +GArray * +gl_journal_update_boots (GlJournal *journal) It seems odd to have a function in the GlJournal API to update the list of boots. Shouldn't GlJournal have all the information (when changing the match string) to update the list arutomatically? Additionally, the code seems to duplicate the code already in gl_journal_get_boots(), whereas it should share it. ::: src/gl-window.c @@ +325,3 @@ + } + + gl_event_toolbar_update_latest_boot_menu_label (toolbar, latest_boot_time); Everything done by gl_event_toolbar_update_latest_boot_menu_label() should be done by calling gl_event_toolbar_change_current_boot(). It's fine to have a separate internal function to update the label, but this should not be exposed in the header (only the toolbar has knowledge of what actions it is appropriate to take when the current boot changes).
Created attachment 326193 [details] [review] Update time after changing category Added support for updating the time when a different category is selected.
Can someone review the above patch ? It could serve as a temporary fix until the feature "** - now" is implemented in bug 753471 as said by Jonathan. Another option can be that I implement "** - now" in the patch related to this bug itself. Any Suggestions ?
Created attachment 346538 [details] [review] patch I generated a patch(works well on my side) quite a while ago, but I forget to attach it here. Here is the patch. This patch update the time of subtitle in toolbar and the time in boot menu.
Created attachment 362523 [details] [review] update boot timestamps when possible
Comment on attachment 362523 [details] [review] update boot timestamps when possible Pushed to master as commit 1f8e848cf7a6a307f9c58637411896ca36ab1517.