ADFA-936: Filter and search logs#1533
Open
dara-abijo-adfa wants to merge 14 commits into
Open
Conversation
Mechanical prep for search & filter in the output tabs: - Wrap the log/build-output editors in a vertical LinearLayout with a ViewStub slot for the upcoming filter bar; the editor keeps its id. - NonEditableEditorFragment now resolves the editor via binding.editor instead of the (formerly editor-typed) layout root. - EditorSearchLayout gains opt-out flags for read-only hosts: hide the replace action and skip the collapsed-sheet bottom margin. Also adds refreshSearch()/isSearchModeActive() so hosts can re-run a query after replacing editor content. Existing call site is unchanged (defaults). - New layout_log_filter_bar.xml (text filter input + level filter chips) and its strings.
…ering LogLine level/tag metadata used to be discarded (and the pooled instance recycled) when a line was flattened to a string in LogViewModel.submit(), which made level filtering impossible and relied on SharedFlow replay to restore logs on re-collection. - New LogFilter (enabled-levels set + case-insensitive text match) and LogBuffer (bounded, thread-safe entry history with sequence numbers, trimmed like the editor: 5000 -> 4700). - LogViewModel now emits snapshot-then-tail UiEvents: on every (re)collection or filter change, a SetText snapshot of the filtered history, then Appends for matching live lines. The snapshot is taken in the live stream's onSubscription and stitched by sequence number, so no line is missed or duplicated. This replaces replay-as-history and also fixes cleared logs resurrecting after rotation. - GlobalBufferAppender.Consumer now receives the logback Level (the appender already stored it); IDELogFragment maps it to ILogger.Level so IDE log lines carry level metadata. - LogViewFragment handles SetText, clears the ViewModel history in clearOutput(), and shares the unfiltered history instead of the (possibly filtered) editor text.
Build output has no level metadata, so it gets the text filter only. The filter applies to the editor view; the session file always receives the unfiltered text, so clearing the filter (or sharing) restores everything. - BuildOutputViewModel: filterText state + pure filterLines() helper. - BuildOutputFragment: re-renders the 512KB editor window from the session file when the filter changes, applies the filter to live batches in flushToEditor() and to the restore path. A mutex serializes re-renders against batch appends so neither misses nor duplicates concurrently flushed output.
- New SearchableOutputFragment interface (beginSearch/toggleFilterBar), implemented by the log fragments and BuildOutputFragment. - Each output tab attaches the existing EditorSearchLayout below its editor (replace hidden, no collapsed-sheet margin) and lazily inflates the filter bar from the ViewStub. LogFilterBarController wires the bar: level chips apply immediately, text input debounced 250ms; level chips are hidden for Build Output. - Active searches refresh (or stop) after a filter re-render replaces the editor content. - Bottom sheet: new mini search/filter FABs shown on Searchable tabs; both expand the sheet before acting since the UI lives inside it. Tooltip tags OUTPUT_SEARCH/OUTPUT_FILTER added.
…peline - LogFilterTest: level sets, null-level pass-through, case-insensitive text match, combined filters. - LogBufferTest: seq monotonicity, filtered snapshots with lastSeq, trim behavior, clear. - LogViewModelTest: snapshot on collection, gap/duplicate-free live tail, filter re-render, ingestion filtering, clear, re-collection idempotence, LogLine capture-before-recycle. - BuildOutputFilterTest: pure filterLines() cases including a batch without a trailing newline.
Mechanical: gradlew spotlessApply over the files changed on this branch.
log_level_verbose/log_level_debug already exist in termux_shared_strings.xml in the same module; resource merging fails on the duplicates. Prefix ours with log_filter_.
uiEvents is now a cold flow whose collect() completes, so the Nothing return type no longer compiles.
GlobalBufferAppender replays its buffer on every registerConsumer(). With the fragment as consumer, each tab switch (view recreation) re-registered and re-submitted the replayed lines into the retained LogBuffer, duplicating history. The activity-scoped IDELogsViewModel is now the consumer, so the replay happens once per ViewModel lifetime. Found during on-device verification (line duplicated after switching tabs away and back).
dara-abijo-adfa
marked this pull request as ready for review
July 17, 2026 11:32
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
📝 WalkthroughWalkthroughChangesFiltered output pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GlobalBufferAppender
participant IDELogsViewModel
participant LogViewModel
participant LogViewFragment
participant EditorBottomSheet
GlobalBufferAppender->>IDELogsViewModel: deliver level-aware log
IDELogsViewModel->>LogViewModel: submit log entry
LogViewModel-->>LogViewFragment: filtered SetText or Append event
EditorBottomSheet->>LogViewFragment: invoke search or filter action
LogViewFragment->>LogViewModel: update filter state
LogViewModel-->>LogViewFragment: render filtered snapshot
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
logger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.kt (1)
62-70: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake buffered replay and live registration atomic.
The consumer is added before
buffer.forEach, so a concurrent append can be dispatched live and then observed again by the weakly consistent queue iterator. It can also arrive before older replayed entries. Introduce an atomic replay-to-live handoff or sequence-based deduplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@logger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.kt` around lines 62 - 70, Update registerConsumer so replaying existing buffer entries and enabling live dispatch occur as one atomic handoff, preventing concurrent appends from being delivered twice or ahead of older replayed messages. Coordinate consumer registration with the append/dispatch path, or use sequence-based deduplication, while preserving ordered delivery of buffered entries followed by new entries.
🧹 Nitpick comments (1)
editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt (1)
92-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove unused
optionsMenuinitialization.This
PopupMenublock is dead code. ThemoreOptionsbutton click listener directly callsshowPopupMenu(...)(which inflates a customPopupWindow), meaningoptionsMenuis never displayed or utilized.
You can safely remove this block along with theoptionsMenuproperty declaration on line 72.♻️ Proposed refactor
- private val optionsMenu: PopupMenu ... - optionsMenu = PopupMenu(context, findInFileBinding.moreOptions, Gravity.TOP) - optionsMenu.menu.add(0, 0, 0, R.string.msg_ignore_case).apply { - isCheckable = true - isChecked = true - } - - optionsMenu.menu.add(0, 1, 0, R.string.msg_use_regex).apply { - isCheckable = true - isChecked = false - } - - optionsMenu.setOnMenuItemClickListener { - return@setOnMenuItemClickListener if (it.isCheckable) { - it.isChecked = !it.isChecked - - val caseInsensitive = searchOptions.caseInsensitive - val regex = searchOptions.type == SearchOptions.TYPE_REGULAR_EXPRESSION - searchOptions = - when (it.itemId) { - 0 -> SearchOptions(it.isChecked, regex) - 1 -> SearchOptions(caseInsensitive, it.isChecked) - else -> searchOptions - } - editor.searcher.updateSearchOptions(searchOptions) - - true - } else { - false - } - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt` around lines 92 - 121, Remove the unused optionsMenu property and its PopupMenu initialization/configuration block in EditorSearchLayout, including the menu item click listener. Preserve the moreOptions click flow that displays the custom PopupWindow via showPopupMenu(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 59-61: Update the timeout/deferred-append path in the editor
content serialization flow around editorContentMutex so it awaits layout and
performs the append while still holding the mutex, rather than launching a
separate coroutine that outlives the lock. Preserve ordering with later batches
and prevent filter re-renders from replacing the editor before the stale
visibleText append completes.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt`:
- Around line 89-91: Replace broad Exception catches in BuildOutputViewModel’s
file-operation blocks with narrow handling: catch IOException for the append,
sessionFile.readText(), range extraction readText(), and RandomAccessFile
interactions, and catch IOException and/or SecurityException for file deletion.
Preserve the existing logging and behavior while allowing unexpected exceptions
to propagate. Apply this at
app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt ranges
89-91, 115-117, 137-139, 153-155, and 176-178.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.kt`:
- Around line 83-118: Update the liveEntries handoff and uiEvents collection to
avoid silently losing entries when the collector falls behind: either use
lossless buffering or detect sequence gaps before filtering and emit a fresh
buffer.snapshotFiltered(filter) SetText snapshot. Preserve sequence-based
stitching and ensure entries retained in LogBuffer are eventually reflected in
the editor.
- Around line 160-176: Serialize the compound operations in LogViewModel:
protect buffer.append followed by liveEntries.tryEmit in submit, and
buffer.clear followed by generation.update in clear, with the same lock. Ensure
clear cannot interleave between append and emission, preventing cleared entries
from being re-emitted while preserving submission order.
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt`:
- Around line 320-328: Replace the broad catch in the regex validation near
EditorSearchLayout.kt lines 320-328 with a catch for
java.util.regex.PatternSyntaxException only. Also refactor the runCatching block
near lines 260-265 into explicit try/catch handling that catches only
PatternSyntaxException, preserving the existing fallback behavior for invalid
patterns at both sites.
In `@logger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.kt`:
- Around line 89-95: Update GlobalBufferAppender.dispatchTo so it no longer uses
runCatching around consumer.consume. Catch only the specific expected exception
type, log handled failures through the existing SLF4J logger, and allow
unexpected or fatal Throwables to propagate.
---
Outside diff comments:
In `@logger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.kt`:
- Around line 62-70: Update registerConsumer so replaying existing buffer
entries and enabling live dispatch occur as one atomic handoff, preventing
concurrent appends from being delivered twice or ahead of older replayed
messages. Coordinate consumer registration with the append/dispatch path, or use
sequence-based deduplication, while preserving ordered delivery of buffered
entries followed by new entries.
---
Nitpick comments:
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt`:
- Around line 92-121: Remove the unused optionsMenu property and its PopupMenu
initialization/configuration block in EditorSearchLayout, including the menu
item click listener. Preserve the moreOptions click flow that displays the
custom PopupWindow via showPopupMenu(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5cb05267-9564-4699-8d24-e54a44175fb8
📒 Files selected for processing (26)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/IDELogFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/NonEditableEditorFragment.javaapp/src/main/java/com/itsaky/androidide/fragments/output/SearchableOutputFragment.ktapp/src/main/java/com/itsaky/androidide/logs/LogBuffer.ktapp/src/main/java/com/itsaky/androidide/models/LogFilter.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/IDELogsViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.ktapp/src/main/res/layout/fragment_log.xmlapp/src/main/res/layout/fragment_non_editable_editor.xmlapp/src/main/res/layout/layout_editor_bottom_sheet.xmlapp/src/main/res/layout/layout_log_filter_bar.xmlapp/src/test/java/com/itsaky/androidide/logs/LogBufferTest.ktapp/src/test/java/com/itsaky/androidide/models/LogFilterTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/BuildOutputFilterTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/LogViewModelTest.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.ktidetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.ktlogger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.ktresources/src/main/res/drawable/ic_keyboard_arrow_up.xmlresources/src/main/res/values/strings.xmlresources/src/main/res/values/styles.xml
Comment on lines
+59
to
+61
| // Serializes editor-content mutations (filtered re-renders vs live batch appends) | ||
| // so a re-render never misses or duplicates a concurrently flushed batch. | ||
| private val editorContentMutex = Mutex() |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep deferred appends inside the serialization boundary.
The timeout branch launches the append and releases editorContentMutex. A filter re-render can then replace the editor before stale visibleText is appended, or later batches can overtake it. Await layout and append directly while retaining the mutex.
Proposed fix
} else {
- viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) {
- editor?.run {
- awaitLayout(onForceVisible = { emptyStateViewModel.setEmpty(false) })
- appendBatch(visibleText)
- emptyStateViewModel.setEmpty(false)
- }
- }
+ awaitLayout(onForceVisible = { emptyStateViewModel.setEmpty(false) })
+ appendBatch(visibleText)
+ emptyStateViewModel.setEmpty(false)
}Also applies to: 267-295
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`
around lines 59 - 61, Update the timeout/deferred-append path in the editor
content serialization flow around editorContentMutex so it awaits layout and
performs the append while still holding the mutex, rather than launching a
separate coroutine that outlives the lock. Preserve ordering with later batches
and prevent filter re-renders from replacing the editor before the stale
visibleText append completes.
Comment on lines
+89
to
+91
| } catch (e: Exception) { | ||
| log.error("Failed to append build output to session file", e) | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use narrow exception handling instead of a broad catch-all.
Multiple blocks performing file operations in this class catch a generic Exception. Based on learnings, prefer narrowing these blocks to specifically catch expected failures, such as IOException (and SecurityException if applicable). Broad catches mask bugs and prevent fail-fast behavior during development.
app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L89-L91: CatchIOExceptioninstead ofExceptionfor the file append failure.app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L115-L117: CatchIOExceptioninstead ofExceptionfor thesessionFile.readText()operation.app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L137-L139: CatchIOExceptioninstead ofExceptionfor thereadText()range extraction.app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L153-L155: CatchIOExceptionand/orSecurityExceptioninstead ofExceptionduring file deletion.app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L176-L178: CatchIOExceptioninstead ofExceptionfor theRandomAccessFileinteractions.
📍 Affects 1 file
app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L89-L91(this comment)app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L115-L117app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L137-L139app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L153-L155app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt#L176-L178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt`
around lines 89 - 91, Replace broad Exception catches in BuildOutputViewModel’s
file-operation blocks with narrow handling: catch IOException for the append,
sessionFile.readText(), range extraction readText(), and RandomAccessFile
interactions, and catch IOException and/or SecurityException for file deletion.
Preserve the existing logging and behavior while allowing unexpected exceptions
to propagate. Apply this at
app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt ranges
89-91, 115-117, 137-139, 153-155, and 176-178.
Source: Learnings
Comment on lines
+83
to
+118
| private val liveEntries = | ||
| MutableSharedFlow<LogBuffer.Entry>( | ||
| replay = 0, | ||
| extraBufferCapacity = EVENT_REPLAY_COUNT, | ||
| extraBufferCapacity = EVENT_BUFFER_COUNT, | ||
| onBufferOverflow = BufferOverflow.DROP_OLDEST, | ||
| ) | ||
|
|
||
| @OptIn(FlowPreview::class) | ||
| val uiEvents: SharedFlow<UiEvent> = | ||
| logs | ||
| .map { if (it.endsWith("\n")) it else "$it\n" } | ||
| .chunkedBySizeOrTime(MAX_CHUNK_SIZE, LOG_FREQUENCY) | ||
| .map<String, UiEvent> { UiEvent.Append(it) } | ||
| .shareIn( | ||
| scope = viewModelScope, | ||
| started = SharingStarted.Eagerly, | ||
| replay = EVENT_REPLAY_COUNT, | ||
| ) | ||
| private val _filter = MutableStateFlow(LogFilter.NONE) | ||
| val filter: StateFlow<LogFilter> = _filter.asStateFlow() | ||
|
|
||
| // Bumped when the buffer is cleared, to force a re-render without a filter change | ||
| private val generation = MutableStateFlow(0) | ||
|
|
||
| /** | ||
| * The log view content as UI events: on every (re)collection or filter change, a | ||
| * [UiEvent.SetText] snapshot of the retained history filtered by the current [filter], | ||
| * followed by [UiEvent.Append]s for matching live lines. The snapshot is taken after | ||
| * subscribing to the live stream and stitched by sequence number, so no line is | ||
| * missed or duplicated in between. | ||
| */ | ||
| @OptIn(ExperimentalCoroutinesApi::class) | ||
| val uiEvents: Flow<UiEvent> = | ||
| combine(_filter, generation) { filter, _ -> filter } | ||
| .flatMapLatest { filter -> | ||
| channelFlow { | ||
| var snapshotSeq = 0L | ||
| liveEntries | ||
| .onSubscription { | ||
| val (text, lastSeq) = buffer.snapshotFiltered(filter) | ||
| snapshotSeq = lastSeq | ||
| send(UiEvent.SetText(text)) | ||
| }.filter { entry -> | ||
| entry.seq > snapshotSeq && filter.matches(entry.level, entry.text) | ||
| }.map { entry -> entry.text } | ||
| .chunkedBySizeOrTime(MAX_CHUNK_SIZE, LOG_FREQUENCY) | ||
| .collect { chunk -> send(UiEvent.Append(chunk)) } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Recover instead of silently dropping live entries.
DROP_OLDEST can create sequence gaps when the collector falls behind. Because the tail only checks entry.seq > snapshotSeq, dropped lines remain in LogBuffer but never reach the editor until a later re-render. Use a lossless handoff or detect sequence gaps and emit a fresh snapshot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.kt` around
lines 83 - 118, Update the liveEntries handoff and uiEvents collection to avoid
silently losing entries when the collector falls behind: either use lossless
buffering or detect sequence gaps before filtering and emit a fresh
buffer.snapshotFiltered(filter) SetText snapshot. Preserve sequence-based
stitching and ensure entries retained in LogBuffer are eventually reflected in
the editor.
Comment on lines
+160
to
+176
| fun submit( | ||
| level: ILogger.Level?, | ||
| line: String, | ||
| ) { | ||
| val text = if (line.endsWith("\n")) line else "$line\n" | ||
| val entry = buffer.append(level, text) | ||
| liveEntries.tryEmit(entry) | ||
| } | ||
|
|
||
| fun setFilter(filter: LogFilter) { | ||
| _filter.value = filter | ||
| } | ||
|
|
||
| /** Clear the retained log history and re-render the (now empty) view. */ | ||
| fun clear() { | ||
| buffer.clear() | ||
| generation.update { it + 1 } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Serialize append, emission, and clear as one operation.
Concurrent submissions can append in sequence order but emit in reverse order. More critically, clear() can run after buffer.append() but before tryEmit(), causing a cleared entry to reappear in the live tail. Guard these compound operations with the same lock or route all commands through one serialized channel.
Proposed synchronization
+ private val eventLock = Any()
+
fun submit(
level: ILogger.Level?,
line: String,
) {
val text = if (line.endsWith("\n")) line else "$line\n"
- val entry = buffer.append(level, text)
- liveEntries.tryEmit(entry)
+ synchronized(eventLock) {
+ val entry = buffer.append(level, text)
+ liveEntries.tryEmit(entry)
+ }
}
fun clear() {
- buffer.clear()
- generation.update { it + 1 }
+ synchronized(eventLock) {
+ buffer.clear()
+ generation.update { it + 1 }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun submit( | |
| level: ILogger.Level?, | |
| line: String, | |
| ) { | |
| val text = if (line.endsWith("\n")) line else "$line\n" | |
| val entry = buffer.append(level, text) | |
| liveEntries.tryEmit(entry) | |
| } | |
| fun setFilter(filter: LogFilter) { | |
| _filter.value = filter | |
| } | |
| /** Clear the retained log history and re-render the (now empty) view. */ | |
| fun clear() { | |
| buffer.clear() | |
| generation.update { it + 1 } | |
| private val eventLock = Any() | |
| fun submit( | |
| level: ILogger.Level?, | |
| line: String, | |
| ) { | |
| val text = if (line.endsWith("\n")) line else "$line\n" | |
| synchronized(eventLock) { | |
| val entry = buffer.append(level, text) | |
| liveEntries.tryEmit(entry) | |
| } | |
| } | |
| fun setFilter(filter: LogFilter) { | |
| _filter.value = filter | |
| } | |
| /** Clear the retained log history and re-render the (now empty) view. */ | |
| fun clear() { | |
| synchronized(eventLock) { | |
| buffer.clear() | |
| generation.update { it + 1 } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.kt` around
lines 160 - 176, Serialize the compound operations in LogViewModel: protect
buffer.append followed by liveEntries.tryEmit in submit, and buffer.clear
followed by generation.update in clear, with the same lock. Ensure clear cannot
interleave between append and emission, preventing cleared entries from being
re-emitted while preserving submission order.
Comment on lines
+320
to
+328
| val query = | ||
| s.toString().let { | ||
| if (searchOptions.type == SearchOptions.TYPE_REGULAR_EXPRESSION) { | ||
| try { | ||
| Pattern.compile(it) | ||
| it | ||
| } catch (_: Throwable) { | ||
| "" | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Prefer narrow exception handling for Pattern.compile.
Based on learnings, in Kotlin files across the AndroidIDE project, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all (like catching Throwable). Both areas use catch-alls when evaluating regex strings; limit them to PatternSyntaxException.
editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt#L320-L328: Changecatch (_: Throwable)tocatch (_: java.util.regex.PatternSyntaxException).editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt#L260-L265: Refactor therunCatchingblock to atry/catchspecifically targetingPatternSyntaxExceptionrather than ignoring all possible failures.
📍 Affects 1 file
editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt#L320-L328(this comment)editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt#L260-L265
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kt`
around lines 320 - 328, Replace the broad catch in the regex validation near
EditorSearchLayout.kt lines 320-328 with a catch for
java.util.regex.PatternSyntaxException only. Also refactor the runCatching block
near lines 260-265 into explicit try/catch handling that catches only
PatternSyntaxException, preserving the existing fallback behavior for invalid
patterns at both sites.
Source: Learnings
Comment on lines
89
to
+95
| private fun dispatchTo( | ||
| consumer: Consumer, | ||
| level: Level, | ||
| message: String | ||
| message: String, | ||
| ) { | ||
| if (level.levelInt < consumer.logLevel.levelInt) return | ||
| runCatching { consumer.consume(message) } | ||
| runCatching { consumer.consume(level, message) } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not silently catch every consumer failure.
runCatching catches all Throwable values and ignores the result, hiding consumer defects and fatal errors. Catch only an expected exception type and log it through SLF4J, or allow unexpected failures to surface.
Based on learnings, Kotlin runtime handling should catch only the specific exception type rather than a broad catch-all. As per coding guidelines, handled exceptions must be logged and catches must be narrow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@logger/src/main/java/com/itsaky/androidide/logging/GlobalBufferAppender.kt`
around lines 89 - 95, Update GlobalBufferAppender.dispatchTo so it no longer
uses runCatching around consumer.consume. Catch only the specific expected
exception type, log handled failures through the existing SLF4J logger, and
allow unexpected or fatal Throwables to propagate.
Sources: Coding guidelines, Learnings
jatezzz
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira tickets - ADFA-936, ADFA-937
Implement Filter and Search within logs -
Build Output,IDE LogsandApp Logs