luci-base,luci-mod-status: make log text search case-insensitive - #8970
luci-base,luci-mod-status: make log text search case-insensitive#8970Ser9ei wants to merge 2 commits into
Conversation
b2cdd7e to
54967fb
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 2 new commits. Both commit messages match their diffs (luci-base → tools/views.js, luci-mod-status → status/dmesg.js), and the two changed sites are the only case-sensitive log-text filters in the tree — logTagFilter was already lowercased, and the facility/severity filters compare against fixed dropdown values. Hoisting toLowerCase() out of the filter callback is the right shape, and both logTextFilter fields are initialised to '' and only ever assigned from an <input>.value, so the unguarded .toLowerCase() in views.js cannot throw.
One inline nit about empty-filter behaviour in CBILogreadBox.
Generated by Claude Code
| const filter = this.logTextFilter.toLowerCase(); | ||
| loglines = loglines.filter(line => { | ||
| const match = line.includes(this.logTextFilter); | ||
| const match = line.toLowerCase().includes(filter); | ||
| return this.invertLogTextSearch ? !match : match; | ||
| }); |
There was a problem hiding this comment.
nit: parity gap with the sister implementation this PR also touches — dmesg.js wraps the same text filter in if (this.logTextFilter) (dmesg.js:97-101),`` but CBILogreadBox runs it unconditionally. With an empty search box and the "Not" checkbox ticked, `line.toLowerCase().includes('')` is always `true`, so the inversion hides every line and the System Log renders empty.
This is pre-existing (it behaved the same before the toLowerCase() change), but since you're already aligning the two views' search semantics here, adding the guard would make them consistent:
| const filter = this.logTextFilter.toLowerCase(); | |
| loglines = loglines.filter(line => { | |
| const match = line.includes(this.logTextFilter); | |
| const match = line.toLowerCase().includes(filter); | |
| return this.invertLogTextSearch ? !match : match; | |
| }); | |
| if (this.logTextFilter) { | |
| const filter = this.logTextFilter.toLowerCase(); | |
| loglines = loglines.filter(line => { | |
| const match = line.toLowerCase().includes(filter); | |
| return this.invertLogTextSearch ? !match : match; | |
| }); | |
| } |
Worth folding into this series or leaving for a separate fix, your call.
Generated by Claude Code
|
Added: Skip empty text filters in luci-base, matching luci-mod-status. Notes on a separate pre-existing issue: While verifying the empty-filter + “Not including” behaviour, I noticed another pre-existing problem in CBILogreadBox (views.js). This is stable and reproducible even when no new messages are being written to the log. It is not caused by the ring buffer simply advancing with new messages. |
This makes text search in the System Log view case-insensitive. Also skip text filtering when the search field is empty Signed-off-by: Sergei S. <it4notice@proton.me>
This makes text search in the Kernel Log view case-insensitive. Signed-off-by: Sergei S. <it4notice@proton.me>
54967fb to
27bb495
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 2 new commits; no new issues found.
The only change since the last review is the if (this.logTextFilter) guard in CBILogreadBox, which is exactly what the earlier nit asked for — the two views' text-search semantics now match, and an empty search box with "Not including" ticked no longer blanks the System Log. Both commit messages still match their diffs, and the luci-base body was updated to mention the added empty-filter skip. CI on the head commit is green.
The unrelated behaviour you described in the PR comment (older entries appearing when a filter control changes) looks like a separate pre-existing issue in the log-fetch path rather than anything introduced here; a follow-up PR or issue is the right place for it.
Generated by Claude Code
Description
Enable case-insensitive text search in the System Log and Kernel Log views.
Related to #8952
Maintainer
@systemcrash, @jow-
Tested on
OpenWrt version: OpenWrt 25.12.5 (r33051-f5dae5ece4)
LuCI version: LuCI openwrt-25.12 branch (26.209.73834~b61f907)
Web browser(s): Firefox 153.0.4
Checklist