After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 709152 - Make use of journal fields in search
Make use of journal fields in search
Status: RESOLVED FIXED
Product: gnome-logs
Classification: Other
Component: general
git master
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-logs maintainer(s)
gnome-logs maintainer(s)
: 743621 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-10-01 03:06 UTC by Matthias Clasen
Modified: 2016-02-22 02:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This is the patch for the bug, but without logical AND or OR, which will be included in the later patch. (4.30 KB, patch)
2015-07-19 14:27 UTC, Jonathan Kang
needs-work Details | Review
patch updated (4.45 KB, patch)
2015-07-20 08:49 UTC, Jonathan Kang
needs-work Details | Review
This one should be fine (4.99 KB, patch)
2015-07-20 09:24 UTC, Jonathan Kang
committed Details | Review
Make use of journal fields in search (4.99 KB, patch)
2015-07-21 02:08 UTC, Jonathan Kang
committed Details | Review
Patch for improving search with boolean combination (12.26 KB, patch)
2015-08-11 11:08 UTC, Jonathan Kang
committed Details | Review
committed patch (2.74 KB, patch)
2016-02-22 02:15 UTC, Jonathan Kang
committed Details | Review

Description Matthias Clasen 2013-10-01 03:06:56 UTC
The search in gnome-logs should be as featureful as the search in journalctl.

We should allow to specify search terms for specific fields and allow to use boolean combinations.

The field names should be available from the ui in some way, similar to how journalctl offers them via commandline completion.
Comment 1 Matthias Clasen 2013-10-01 03:21:18 UTC
Oh, and just like journalctl does, it would also be really nice to have an equivalent for the completion functionality for field values, not just field names.
Comment 2 David King 2015-01-28 09:36:42 UTC
*** Bug 743621 has been marked as a duplicate of this bug. ***
Comment 3 Lars Karlitski 2015-02-15 18:25:56 UTC
Right now we're only searching through the live objects that are currently in 
the list box.

When we want to let people search for arbitrary fields, we'll need to change that to always consult the journal for a search. I'd prefer that, but it will probably make searching a bit slower for people with slow disks.
Comment 4 Jonathan Kang 2015-07-19 14:27:33 UTC
Created attachment 307678 [details] [review]
This is the patch for the bug, but without logical AND or OR, which will be included in the later patch.
Comment 5 David King 2015-07-20 07:43:27 UTC
Review of attachment 307678 [details] [review]:

This patch breaks regular searching for me, as I can no longer enter a search term and have it match on the message text.

::: src/gl-eventviewlist.c
@@ +222,3 @@
+            }
+
+            matches = (utf8_strcasestr ("_comm", field_name) &&

You should use strstr() and the field name (as above) here, rather than using utf8_strcaststr().
Comment 6 Jonathan Kang 2015-07-20 08:49:16 UTC
Created attachment 307734 [details] [review]
patch updated
Comment 7 David King 2015-07-20 09:04:26 UTC
Review of attachment 307734 [details] [review]:

I still cannot search as normal (without giving the journal fields), which I could do before applying this patch.

::: src/gl-eventviewlist.c
@@ +208,3 @@
+            }
+
+            matches = (strstr ("_comm", field_name) &&

Oh, I missed that this string comes from the search field in the UI. In that case, utf8_strcasestr() is fine. Sorry!
Comment 8 Jonathan Kang 2015-07-20 09:06:18 UTC
Yep, I'm fixing the search to make it work as before without giving journal fields.
Comment 9 Jonathan Kang 2015-07-20 09:24:58 UTC
Created attachment 307738 [details] [review]
This one should be fine
Comment 10 David King 2015-07-20 11:11:08 UTC
Review of attachment 307738 [details] [review]:

Looks good now! Please push to master.
Comment 11 Jonathan Kang 2015-07-21 02:08:30 UTC
Created attachment 307791 [details] [review]
Make use of journal fields in search
Comment 12 Jonathan Kang 2015-07-21 02:43:36 UTC
(In reply to Jonathan Kang from comment #11)
> Created attachment 307791 [details] [review] [review]
> Make use of journal fields in search

Sorry for the wrong action :(

Pushed the patch to master as dd84d64262547388a5b1fe9f7bb1bbc7cb26153a. :)
Comment 13 Jonathan Kang 2015-08-11 11:08:21 UTC
Created attachment 309056 [details] [review]
Patch for improving search with boolean combination

With this patch, while searching, field names are needed like search of journalctl. Later patch will improve this. And cached tokenized string will be used in a later patch.
Comment 14 David King 2015-08-11 11:25:11 UTC
Review of attachment 309056 [details] [review]:

Patch looks good overall. Can you mention somewhere about the limitations with the search syntax (like requiring the full journal fields), and also how it differs from the journalctl syntax? We should probably get the help updated too (in a separate bug and/or patch).

::: src/gl-eventviewlist.c
@@ +58,3 @@
+{
+    LOGICAL_OR = 2,
+    LOGICAL_AND = 3

Is there any reason to set these enum values, or can they just be unset?

@@ +183,3 @@
+    gchar *field_value;
+    gint i;
+    gint match_stack[10];

Is match_stach a gint or ElEventViewListLogic?

@@ +185,3 @@
+    gint match_stack[10];
+    gint match_count = 0;
+    gint token_index = 0;

Do these ever drop to less than 0? Otherwise, please make them guints.

@@ +219,3 @@
+                  (message && utf8_strcasestr (message, search_text)) ||
+                  (kernel_device && utf8_strcasestr (kernel_device, search_text)) ||
+                  (audit_session && utf8_strcasestr (audit_session, search_text));

Another possible point of future optimisation is to cache the casefolded search_text.

@@ +291,3 @@
+    }
+
+    if (match_count > 2)

Would be good to have a comment here (about what a match count greater than 2 implies), and also in a few other places.
Comment 15 Jonathan Kang 2015-08-11 13:40:15 UTC
Pushed to master as e73c75620fe51a6a45469697ed2c8ee9e5354bad and 717e0092d8f789fe6931a9b4ef9437f925fa7e22.
Comment 16 Jonathan Kang 2016-02-22 02:15:33 UTC
Created attachment 321800 [details] [review]
committed patch

committed patch for commit717e0092d8f789fe6931a9b4ef9437f925fa7e22
Comment 17 Jonathan Kang 2016-02-22 02:17:09 UTC
Closing this bug.