Created attachment 309758 [details] [review]
patch

Output logs shown in the current GtkListBox

Review of attachment 309758 [details] [review]:

As a general comment, it is better to say "export" rather than "output", as the process is in only one direction (and currently there is no import).

::: data/gl-eventtoolbar.ui
@@ +25,3 @@
+            <object class="GtkButton" id="output_button">
+                <property name="action-name">win.output</property>
+                <property name="tooltip-text" translatable="yes">Click to output logs</property>

A better tooltip would be "Export logs to a file".

::: src/gl-eventviewlist.c
@@ +114,3 @@
+
+            buffer_tmp = output_buf;
+            output_buf = g_strconcat (buffer_tmp, output_text, NULL);

The g_strconcat() here is going to use lots of allocations, to continually reallocate the buffer. A better approach is to use GMemoryOutputStream, which can then be spliced into the output stream when writing to a file (simplifying that code). This also avoids the g_strdup() calls, as anything written to the GMemoryOutputStream is copied.

::: src/gl-window.c
@@ +192,3 @@
+    file_chooser = GTK_FILE_CHOOSER (dialog);
+    gtk_file_chooser_set_do_overwrite_confirmation (file_chooser, TRUE);
+    gtk_file_chooser_set_current_name (file_chooser, "Untitled document");

That filename should be translatable, and something more useful would be better. The best string would be something which gave the timestamps of the start and end of the exported messages, and the category. That is somewhat unrelated to this patch, so just something like "log messages" would be fine for the moment.

@@ +213,3 @@
+                                                   GTK_MESSAGE_ERROR,
+                                                   GTK_BUTTONS_OK,
+                                                   "%s",

All the message dialogues should use GTK_BUTTONS_CLOSE (as there is nothing that the user can do other than dismissing the dialogue). We can also do better than just presenting the error (in fact, we should probably avoid presenting it at all, as it is unlikely to be meaningful for the user.

@@ +214,3 @@
+                                                   GTK_BUTTONS_OK,
+                                                   "%s",
+                                                   error->message);

Let's only use g_warning to log the error (as you do below), and add a (translated) message here saying "Unable to export log messages to a file".

@@ +218,3 @@
+            gtk_widget_destroy (error_dialog);
+
+            g_warning ("%s", error->message);

A better string would give some context: "Error while replacing exported log messages file" %s".

@@ +226,3 @@
+        if (error != NULL)
+        {
+            error_dialog = gtk_message_dialog_new (GTK_WINDOW (user_data),

Rather than (potentially) presenting three dialogues to the user, use g_warning() to output the error message each time, and present a single dialogue at the end if there was an error.

@@ +355,3 @@
     { "search", on_action_toggle, NULL, "false", on_search },
     { "view-boot", on_view_boot, "s", NULL, NULL },
+    { "output", on_output },

export, and on_export, woiuld be better names.

Created attachment 309824 [details] [review]
patch updated

Review of attachment 309824 [details] [review]:

::: src/gl-eventviewlist.c
@@ +126,3 @@
+    buffer = g_memory_output_stream_get_data (G_MEMORY_OUTPUT_STREAM (stream));
+
+    g_output_stream_close (stream, NULL, &error);

You also need to unref the GMemoryOutputStream, or you have a leak.

@@ +134,3 @@
+    }
+
+    return buffer;

It is simpler to return the GMemoryOutputStream here (or better would be the GBytes returned from g_memory_output_stream_steal_as_bytes(), as then you can use g_output_stream_write_bytes() on it directly), both of which have the size included. This saves having to examine the whole buffer again just to check the length (which is already known).

Created attachment 309826 [details] [review]
patch updated

Created attachment 309827 [details] [review]
patch

fixed a leak.

Review of attachment 309827 [details] [review]:

Looks good to me now. As we are past the freeze, and this is a signficant chunk of new code and UI, we should not push this to master yet. It can be pushed once the new development cycle opens (or once there is a stable gnome-3-18 branch).

Review of attachment 309827 [details] [review]:

Pushed to master a while ago as commit a0b01ba05eaf1153a27a05a4eb7f926e34741bfe.