feat: optional match mode for the filter tag and content (#240)#246
feat: optional match mode for the filter tag and content (#240)#246salmon-21 wants to merge 2 commits into
Conversation
9bbc293 to
b04e3d7
Compare
| // Database-local copy of the filters-domain TagMatchMode, so this module stays free of the filters | ||
| // feature (mirrors CrashType). filters:impl maps between the two by name. Persisted by ordinal via | ||
| // TagMatchModeConverter, so the order of these entries must never change. | ||
| enum class TagMatchMode { |
There was a problem hiding this comment.
lets have it named more "broad?" as it can be applied not only to tag
There was a problem hiding this comment.
Renamed TagMatchMode → MatchMode in both the filters and database modules, so it's no longer tied to a single field.
| enum class TagMatchMode { | ||
| CONTAINS, | ||
| REGEX, | ||
| EXACT_LEGACY, |
There was a problem hiding this comment.
lets have it just EXACT, no point in naming it legacy
There was a problem hiding this comment.
Done — it's now just EXACT, and a first-class user-selectable mode (no longer migration-only).
| val tid: String? = null, | ||
| val packageName: String? = null, | ||
| val tag: String? = null, | ||
| val tagMatchMode: TagMatchMode = TagMatchMode.CONTAINS, |
There was a problem hiding this comment.
lets make it not tag only, it can be applied to any string field
easier to have some data class that will have value inside with match mode
smth like
// idk about naming
val tag: MatchData
data class MatchData(
val value: String? = null,
val matchMode: TagMatchMode = TagMatchMode.CONTAINS,
)There was a problem hiding this comment.
Done — the tag is now MatchData(value, matchMode) as you suggested. MatchData is field-agnostic so it can be reused for any string field later; for this PR only the tag adopts it (matching #240's scope).
There was a problem hiding this comment.
Following up on this — MatchData is now adopted by the content field too, not just tag, mirroring logcat where the message supports regex (adb logcat -e). Both tag and content can be matched by contains / regex / exact; the identity fields (uid/pid/tid/packageName) stay exact-match. PR title updated to reflect this.
| // doesn't break distinctUntilChanged / DiffUtil). Null when not in regex mode, or when the | ||
| // pattern is invalid — in which case the filter matches nothing instead of crashing the pipeline. | ||
| @delegate:GsonSkip | ||
| val tagRegex: Regex? by lazy(LazyThreadSafetyMode.PUBLICATION) { |
There was a problem hiding this comment.
looks kinda bad, idk how to make it better, i guess it should be inside data class as a field, not created by lazy ...
There was a problem hiding this comment.
Moved the compiled regex into MatchData as you suggested. I kept it as a lazy member rather than an eager field for two reasons:
- Gson import correctness — Gson deserializes via the all-defaults no-arg constructor and sets
value/matchModereflectively afterwards, so an eager field would compile from the default (null) value before they're populated and end up stale on import. Lazy defers compilation until after they're set. - Performance — lazy caches the compile per instance, which matters since
matches()runs once per log line in the filter pipeline.
It's excluded from equals/hashCode (not a constructor property) and from Gson. Happy to switch to compute-on-demand if you'd prefer.
| packageName = command.filter.packageName, | ||
| tag = command.filter.tag, | ||
| // The UI toggle is binary, so a migration-only EXACT_LEGACY filter loads as | ||
| // non-regex and will persist as CONTAINS if the user re-saves it (intentional). |
There was a problem hiding this comment.
i think this is not what user expects. Lets add full support for every match mode
There was a problem hiding this comment.
Done — replaced the binary regex toggle with full support for all three modes (Contains / Regex / Exact), selectable via a single-choice dialog on the tag field.
| ) { | ||
| // The tag is an invalid regex only when regex mode is on, the tag is non-blank, and it fails to | ||
| // compile. An invalid pattern blocks saving. | ||
| val tagRegexError: Boolean |
There was a problem hiding this comment.
this must be part of state (maybe viewstate), not a just computed variable
There was a problem hiding this comment.
Done — removed the computed canSave/tagRegexError from the state. They're now derived in the view-state mapper and carried as plain fields on the view state; the reducer's save guard computes validity inline from the same MatchData.
aa946e7 to
8a02453
Compare
9cb96a1 to
434e1ff
Compare
The filter tag and content can now each be matched by substring (default), regular expression, or exact equality, chosen from a dialog on the field. This mirrors logcat, where messages support regex and exact/substring matching. Implements review feedback on the original regex proposal: - Model the matched value as a reusable MatchData(value, matchMode) instead of a bare String plus a sibling match-mode flag, so match modes aren't wired to a single field — applied to tag and content. - Name the enum MatchMode (not tag-specific) and use a plain EXACT that is a first-class, user-selectable mode rather than EXACT_LEGACY. - Keep the compiled Regex inside MatchData (lazy, excluded from equals/hashCode and from Gson). Lazy is required for correct Gson import and caches the compile for the filtering hot path. - Offer all three modes through a single-choice dialog (matching the app's other value pickers) instead of a binary regex toggle. - Derive the per-field regex error / canSave in the view-state mapper rather than as computed properties on the state. The DB stores tag/content plus their match-mode columns (migrations 18->19 and 19->20); MatchData is assembled in the filters mapper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
434e1ff to
73b9fe5
Compare
Filter exports from releases before match modes wrote tag/content as plain JSON strings; the model now expects MatchData objects, so importing an old file failed with a JsonSyntaxException and the filters were silently dropped. A Gson TypeAdapterFactory upgrades legacy string fields on read, preserving each field's old matching: tag -> EXACT, content -> CONTAINS, mirroring the database migrations added with MatchData. It delegates to Gson's reflective adapter (no recursion) and is registered on an import-scoped Gson so the shared app Gson stays generic. Current object-shaped exports pass through unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Closes #240. A filter's
tagwas matched by exact string equality, so watching a class whose tag is auto-derived (Timber'sDebugTree, SLF4J-android, etc.) required one filter row per variant — e.g.SyncRepository,SyncRepository$fullSync,SyncRepository$deltaSyncneeded three separate filters. This adds a per-filter regex toggle on the tag field so one pattern (^Sync.*) catches them all.Behavior
containssubstring match — what users expect from a text search box.Regexand matched withcontainsMatchIn.Zero-regression migration (v17 → v18)
Switching the tag default from exact-equality to
containsis a behavior change, so existing filters must keep matching the same lines. Instead of rewriting tag text to^escaped$(lossy, mutates user data), match mode is modeled as an enum and the migration marks every pre-existing tag filter asEXACT_LEGACY:EXACT_LEGACYkeeps the old==semantics, is produced only by the migration, and is never offered in the UI. The tag text is left untouched.Verified on-device: seeded a v17 DB with a tag filter, upgraded to v18 — no crash, the row became
tag_match_mode = 2(EXACT_LEGACY), a null-tag row stayed0(CONTAINS), andidentity_hashupdated to the v18 schema. Old filters keep matching exactly as before.Implementation notes
Regexis cached lazily onUserFilter(a bodyval, so it stays out ofequals/copy/Gson and doesn't breakdistinctUntilChanged/DiffUtil). It compiles once per filter instance, never per log line.@TypeConverterwith a database-localTagMatchModeenum, mirroring the existingCrashTypepattern;filters:implbridges the two enums by name. Thedatabasemodule stays free of the filters domain.Scope is tag only for this PR.
contentalready doescontains, so a regex toggle there is a trivial migration-free follow-up.File-import backward compatibility
Moving
tag/contentfrom plain strings toMatchDataobjects also changes the exported JSON shape. Filter files exported by older releases store"tag":"ActivityThread"(a string), and import deserialized straight intoList<UserFilter>, so those files would now fail with aJsonSyntaxExceptionand the filters would be silently dropped.A Gson
TypeAdapterFactoryupgrades legacy string fields on read, preserving each field's old matching to match the DB migrations: tag → EXACT, content → CONTAINS. It delegates to Gson's reflective adapter (no recursion) and is registered on an import-scoped Gson, so the shared app Gson stays generic. Current object-shaped exports pass through unchanged.Verified on-device (SM-S948Q / Android 16): imported a legacy-shaped file →
tagbecame EXACT,contentbecame CONTAINS (matching the migration); imported a current object-shaped file → match modes preserved (REGEX/EXACT). No regression on new exports.Test plan
:app:assembleDebugbuilds clean on Windows + JDK 22.^Sync.*(regex on) matchesSyncRepositoryandSyncRepository$fullSyncfrom a single filter.[) → inline "Invalid regex", save blocked.🤖 Generated with Claude Code