ADFA-4922: Disable text action in search - #1605
Conversation
![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.
📝 Walkthrough
WalkthroughThe change separates source emptiness from filtered layout emptiness. Output views now preserve filter state, prevent stale renders, and report no-match results. Editor actions observe source content and suppress menus during search. ChangesEmpty-state and interaction feedback
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt (1)
64-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNotify
onVisibilityChangedfor the initial shown state too.
toggle()andhide()notifyonVisibilityChanged, butinitdoes not notify it when the bar first becomes visible afterstub.inflate(). The filter bar layout has no explicitandroid:visibility="gone", socreateFilterBar()in both fragments updatesisFilterActivewithout triggeringupdateEmptyState, leaving the empty-state UI stale until a later toggle or filter change runs it.Invoke
onVisibilityChanged?.invoke(binding.root.isVisible)once after the filter bar state finishes initializing ininit.🤖 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/LogFilterBarController.kt` around lines 64 - 80, Update LogFilterBarController.init to invoke onVisibilityChanged with binding.root.isVisible once after all filter bar initialization and listeners are configured, ensuring the initial visible state is propagated without changing toggle() or hide() behavior.
🧹 Nitpick comments (5)
editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt (2)
97-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument and test the new no-match contract.
isSearchCompleteWithNoMatches()is a new public API. Add KDoc that defines the valid-query, valid-result, and empty-result conditions. Add tests for no query, invalid results, empty results, and non-empty results.As per coding guidelines, public functions and non-obvious logic must have KDoc, and changed non-UI logic needs unit coverage.
Proposed KDoc
+ /** + * Returns true when the current non-empty query has valid results and no matches. + */ fun isSearchCompleteWithNoMatches(): Boolean {🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` around lines 97 - 101, The new public method is missing documentation and unit coverage for its no-match contract. Add KDoc to IDEEditorSearcher.isSearchCompleteWithNoMatches() describing that it returns true only when a query exists, the search result is valid, and the result set is empty; add tests covering no query, invalid results, empty results, and non-empty results.Source: Coding guidelines
35-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the reflection catch.
IDEEditorSearcher.getEditor()uses reflection, so catch the reflection failures you can meaningfully translate, such asNoSuchFieldException,IllegalAccessException,IllegalArgumentException, andClassCastException. CatchingThrowablealso traps fatalErrorvalues and unrelated failures like the assertion-style failures in IDEEditorSearcher; let those preserve their exact shape.🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` around lines 35 - 40, Update IDEEditorSearcher.getEditor() to catch only the reflection and cast failures it can translate: NoSuchFieldException, IllegalAccessException, IllegalArgumentException, and ClassCastException. Keep the existing RuntimeException wrapping for those failures, while allowing fatal Error values and unrelated assertion-style failures to propagate unchanged.Source: Learnings
app/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.kt (1)
49-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
setEmptyStatecontract.Add KDoc that defines
isEmptyas layout state andisSourceEmptyas source-data state. Callers can otherwise invert these values.As per coding guidelines, “Public classes, functions, and non-obvious logic must have KDoc documenting contracts...”.
🤖 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/EmptyStateFragmentViewModel.kt` around lines 49 - 54, Document the public setEmptyState function with KDoc, explicitly defining isEmpty as the layout state and isSourceEmpty as the source-data state. Describe both parameters so callers pass the values without confusing or reversing their meanings.Source: Coding guidelines
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse JUnit Jupiter and Truth for the new tests.
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt#L20-L22: replace JUnit 4 imports with JUnit Jupiter and Truth imports.app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt#L23-L23: replace the added JUnit 4 assertion import.app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt#L98-L108: migrate the new test to JUnit Jupiter and Truth. Convert the enclosing class if required.The existing test dependencies include JUnit Jupiter and Truth, but these tests still use JUnit 4 imports/assertions instead of the required test framework.
🤖 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/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt` around lines 20 - 22, The new tests use JUnit 4 assertions instead of the required JUnit Jupiter and Truth APIs. In app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt lines 20-22, replace the assertion imports with JUnit Jupiter and Truth imports; in app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt line 23, replace the added JUnit 4 assertion import; and in lines 98-108, migrate the new test and enclosing class as needed to JUnit Jupiter and Truth while preserving its assertions and behavior.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (1)
65-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated filter-active/no-match wiring. Both fragments independently declare a
noMatchTracker = FilterNoMatchTracker()and anisFilterActivegetter with the same "filter source is non-empty or bar is visible" shape; this duplication is new in this PR and the two copies can drift.
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt#L65-L69: extractnoMatchTrackerand theisFilterActivepattern (parameterized bybuildOutputViewModel.filterText.value.isNotEmpty()) into a shared helper.app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt#L148-L152: reuse the same shared helper, parameterized byviewModel.filter.value != LogFilter.NONE.🤖 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 65 - 69, Extract the duplicated no-match tracking and filter-active logic into a shared helper, parameterized by the filter-source predicate and the relevant filter-bar visibility. Update app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt lines 65-69 to use it with buildOutputViewModel.filterText.value.isNotEmpty(), and app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt lines 148-152 to use it with viewModel.filter.value != LogFilter.NONE; both sites should reuse the shared noMatchTracker and isFilterActive behavior.
🤖 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/ui/EditorBottomSheet.kt`:
- Around line 241-248: Update the share completion cleanup around shareJob so it
clears the shared state and calls updateActionButtonsEnabledState only when
shareJob still references the current coroutine job. Preserve the current-tab
re-read and empty-state calculation, but prevent an older cancelled share from
clearing or enabling actions for a newer share.
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Around line 48-58: Update IDEEditorSearcher.search and the
navigation/replacement overrides that call markSearching before superclass
validation to stop immediately for empty queries, and reset isSearching when the
superclass throws for invalid patterns. Keep the searching state aligned with
Sora validation while preserving normal successful search behavior.
- Around line 97-101: Update IDEEditorSearcher.isSearchCompleteWithNoMatches()
to rely on the current query’s published search result rather than
isResultValid() and potentially stale lastResults. Track or validate a
query/result generation through the result publication lifecycle, and only
report no matches after the current query’s result has been published; return
false while pending, cancelled, or associated with an older query.
In `@resources/src/main/res/values/strings.xml`:
- Line 689: Update the msg_no_filter_matches string resource to use
output-neutral wording, such as “No matching entries found.”, so it is
appropriate for both log and build-output panels.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt`:
- Around line 64-80: Update LogFilterBarController.init to invoke
onVisibilityChanged with binding.root.isVisible once after all filter bar
initialization and listeners are configured, ensuring the initial visible state
is propagated without changing toggle() or hide() behavior.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 65-69: Extract the duplicated no-match tracking and filter-active
logic into a shared helper, parameterized by the filter-source predicate and the
relevant filter-bar visibility. Update
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt
lines 65-69 to use it with buildOutputViewModel.filterText.value.isNotEmpty(),
and app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt
lines 148-152 to use it with viewModel.filter.value != LogFilter.NONE; both
sites should reuse the shared noMatchTracker and isFilterActive behavior.
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.kt`:
- Around line 49-54: Document the public setEmptyState function with KDoc,
explicitly defining isEmpty as the layout state and isSourceEmpty as the
source-data state. Describe both parameters so callers pass the values without
confusing or reversing their meanings.
In
`@app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt`:
- Around line 20-22: The new tests use JUnit 4 assertions instead of the
required JUnit Jupiter and Truth APIs. In
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt
lines 20-22, replace the assertion imports with JUnit Jupiter and Truth imports;
in app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt line 23,
replace the added JUnit 4 assertion import; and in lines 98-108, migrate the new
test and enclosing class as needed to JUnit Jupiter and Truth while preserving
its assertions and behavior.
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Around line 97-101: The new public method is missing documentation and unit
coverage for its no-match contract. Add KDoc to
IDEEditorSearcher.isSearchCompleteWithNoMatches() describing that it returns
true only when a query exists, the search result is valid, and the result set is
empty; add tests covering no query, invalid results, empty results, and
non-empty results.
- Around line 35-40: Update IDEEditorSearcher.getEditor() to catch only the
reflection and cast failures it can translate: NoSuchFieldException,
IllegalAccessException, IllegalArgumentException, and ClassCastException. Keep
the existing RuntimeException wrapping for those failures, while allowing fatal
Error values and unrelated assertion-style failures to propagate unchanged.
🪄 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 Plus
Run ID: 9197f347-cf3d-42e3-a005-ac4f6e2e49ac
📒 Files selected for processing (18)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/FilterNoMatchTracker.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/logs/LogBuffer.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.ktapp/src/test/java/com/itsaky/androidide/fragments/output/FilterNoMatchTrackerTest.ktapp/src/test/java/com/itsaky/androidide/logs/LogBufferTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/LogViewModelTest.ktcommon/src/main/java/com/itsaky/androidide/utils/KeyboardUtils.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorActionsMenu.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kteditor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.ktresources/src/main/res/values/strings.xml
| override fun search( | ||
| query: String, | ||
| searchOptions: SearchOptions, | ||
| ) { | ||
| if (query.isNotEmpty()) { | ||
| markSearching() | ||
| } else { | ||
| isSearching = false | ||
| } | ||
| super.search(query, searchOptions) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
artifact="$(find "$HOME/.gradle/caches" -type f \( -iname '*sora*.aar' -o -iname '*sora*.jar' \) -print -quit)"
if [[ -z "$artifact" ]]; then
echo "No resolved Sora artifact found" >&2
exit 2
fi
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
if [[ "$artifact" == *.aar ]]; then
unzip -p "$artifact" classes.jar > "$tmp/classes.jar"
else
cp "$artifact" "$tmp/classes.jar"
fi
javap -classpath "$tmp/classes.jar" -c -p io.github.rosemoe.sora.widget.EditorSearcher |
rg -n -C 10 'search|stopSearch|gotoNext|gotoPrevious|checkState'
rg -n -C 5 \
'\bsearcher\.(search|replaceAll|replaceThis|gotoNext|gotoPrevious)\(' \
--glob '*.{kt,java}' .Repository: appdevforall/CodeOnTheGo
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching IDEEditorSearcher / EditorSearcher:"
fd -i 'EditorSearcher|IDEEditorSearcher' . || true
target="$(fd -i 'IDEEditorSearcher' . | head -n 1 || true)"
echo "target=$target"
if [[ -n "$target" ]]; then
wc -l "$target"
ast-grep outline "$target" --view compact || true
cat -n "$target"
fi
echo "\nSora dependency mentions:"
rg -n "sora|editor|io.github.rosemoe.sora" --glob '*.toml' --glob '*.gradle*' --glob '*.properties' . || true
echo "\nUsages of searcher.*search(...):"
rg -n -C 5 'searcher\.(search\(|replaceAll\(|replaceThis\(|gotoNext\(|gotoPrevious\()' --glob '*.{kt,java}' . || trueRepository: appdevforall/CodeOnTheGo
Length of output: 13503
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching IDEEditorSearcher / EditorSearcher:"
fd -i 'IDEEditorSearcher|EditorSearcher' . || true
target="$(fd -i 'IDEEditorSearcher' . | head -n 1 || true)"
echo "target=$target"
if [[ -n "$target" ]]; then
wc -l "$target"
ast-grep outline "$target" --view compact || true
cat -n "$target"
fi
echo "\nSora dependency mentions:"
rg -n "sora|editor|io.github.rosemoe.sora" --glob '*.toml' --glob '*.gradle*' --glob '*.properties' . || true
echo "\nUsages of searcher.*search(...):"
rg -n -C 5 'searcher\.(search\(|replaceAll\(|replaceThis\(|gotoNext\(|gotoPrevious\()' --glob '*.{kt,java}' . || trueRepository: appdevforall/CodeOnTheGo
Length of output: 13503
Keep isSearching aligned with Sora validation.
If super.search(query, searchOptions) validates empty or invalid patterns and throws, this method can leave isSearching == true while the search has no active query/update path. Stop the search explicitly for empty query inputs and reset isSearching in a catch block if Sora throws on invalid regular expressions. This also affects the navigation/replacement overrides because those also call markSearching() before superclass validation.
🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`
around lines 48 - 58, Update IDEEditorSearcher.search and the
navigation/replacement overrides that call markSearching before superclass
validation to stop immediately for empty queries, and reset isSearching when the
superclass throws for invalid patterns. Keep the searching state aligned with
Sora validation while preserving normal successful search behavior.
Source: Coding guidelines
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt (1)
64-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNotify
onVisibilityChangedfor the initial shown state too.
toggle()andhide()notifyonVisibilityChanged, butinitdoes not notify it when the bar first becomes visible afterstub.inflate(). The filter bar layout has no explicitandroid:visibility="gone", socreateFilterBar()in both fragments updatesisFilterActivewithout triggeringupdateEmptyState, leaving the empty-state UI stale until a later toggle or filter change runs it.Invoke
onVisibilityChanged?.invoke(binding.root.isVisible)once after the filter bar state finishes initializing ininit.🤖 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/LogFilterBarController.kt` around lines 64 - 80, Update LogFilterBarController.init to invoke onVisibilityChanged with binding.root.isVisible once after all filter bar initialization and listeners are configured, ensuring the initial visible state is propagated without changing toggle() or hide() behavior.
🧹 Nitpick comments (5)
editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt (2)
97-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument and test the new no-match contract.
isSearchCompleteWithNoMatches()is a new public API. Add KDoc that defines the valid-query, valid-result, and empty-result conditions. Add tests for no query, invalid results, empty results, and non-empty results.As per coding guidelines, public functions and non-obvious logic must have KDoc, and changed non-UI logic needs unit coverage.
Proposed KDoc
+ /** + * Returns true when the current non-empty query has valid results and no matches. + */ fun isSearchCompleteWithNoMatches(): Boolean {🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` around lines 97 - 101, The new public method is missing documentation and unit coverage for its no-match contract. Add KDoc to IDEEditorSearcher.isSearchCompleteWithNoMatches() describing that it returns true only when a query exists, the search result is valid, and the result set is empty; add tests covering no query, invalid results, empty results, and non-empty results.Source: Coding guidelines
35-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the reflection catch.
IDEEditorSearcher.getEditor()uses reflection, so catch the reflection failures you can meaningfully translate, such asNoSuchFieldException,IllegalAccessException,IllegalArgumentException, andClassCastException. CatchingThrowablealso traps fatalErrorvalues and unrelated failures like the assertion-style failures in IDEEditorSearcher; let those preserve their exact shape.🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` around lines 35 - 40, Update IDEEditorSearcher.getEditor() to catch only the reflection and cast failures it can translate: NoSuchFieldException, IllegalAccessException, IllegalArgumentException, and ClassCastException. Keep the existing RuntimeException wrapping for those failures, while allowing fatal Error values and unrelated assertion-style failures to propagate unchanged.Source: Learnings
app/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.kt (1)
49-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
setEmptyStatecontract.Add KDoc that defines
isEmptyas layout state andisSourceEmptyas source-data state. Callers can otherwise invert these values.As per coding guidelines, “Public classes, functions, and non-obvious logic must have KDoc documenting contracts...”.
🤖 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/EmptyStateFragmentViewModel.kt` around lines 49 - 54, Document the public setEmptyState function with KDoc, explicitly defining isEmpty as the layout state and isSourceEmpty as the source-data state. Describe both parameters so callers pass the values without confusing or reversing their meanings.Source: Coding guidelines
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse JUnit Jupiter and Truth for the new tests.
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt#L20-L22: replace JUnit 4 imports with JUnit Jupiter and Truth imports.app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt#L23-L23: replace the added JUnit 4 assertion import.app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt#L98-L108: migrate the new test to JUnit Jupiter and Truth. Convert the enclosing class if required.The existing test dependencies include JUnit Jupiter and Truth, but these tests still use JUnit 4 imports/assertions instead of the required test framework.
🤖 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/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt` around lines 20 - 22, The new tests use JUnit 4 assertions instead of the required JUnit Jupiter and Truth APIs. In app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt lines 20-22, replace the assertion imports with JUnit Jupiter and Truth imports; in app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt line 23, replace the added JUnit 4 assertion import; and in lines 98-108, migrate the new test and enclosing class as needed to JUnit Jupiter and Truth while preserving its assertions and behavior.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (1)
65-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated filter-active/no-match wiring. Both fragments independently declare a
noMatchTracker = FilterNoMatchTracker()and anisFilterActivegetter with the same "filter source is non-empty or bar is visible" shape; this duplication is new in this PR and the two copies can drift.
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt#L65-L69: extractnoMatchTrackerand theisFilterActivepattern (parameterized bybuildOutputViewModel.filterText.value.isNotEmpty()) into a shared helper.app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt#L148-L152: reuse the same shared helper, parameterized byviewModel.filter.value != LogFilter.NONE.🤖 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 65 - 69, Extract the duplicated no-match tracking and filter-active logic into a shared helper, parameterized by the filter-source predicate and the relevant filter-bar visibility. Update app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt lines 65-69 to use it with buildOutputViewModel.filterText.value.isNotEmpty(), and app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt lines 148-152 to use it with viewModel.filter.value != LogFilter.NONE; both sites should reuse the shared noMatchTracker and isFilterActive behavior.
🤖 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/ui/EditorBottomSheet.kt`:
- Around line 241-248: Update the share completion cleanup around shareJob so it
clears the shared state and calls updateActionButtonsEnabledState only when
shareJob still references the current coroutine job. Preserve the current-tab
re-read and empty-state calculation, but prevent an older cancelled share from
clearing or enabling actions for a newer share.
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Around line 48-58: Update IDEEditorSearcher.search and the
navigation/replacement overrides that call markSearching before superclass
validation to stop immediately for empty queries, and reset isSearching when the
superclass throws for invalid patterns. Keep the searching state aligned with
Sora validation while preserving normal successful search behavior.
- Around line 97-101: Update IDEEditorSearcher.isSearchCompleteWithNoMatches()
to rely on the current query’s published search result rather than
isResultValid() and potentially stale lastResults. Track or validate a
query/result generation through the result publication lifecycle, and only
report no matches after the current query’s result has been published; return
false while pending, cancelled, or associated with an older query.
In `@resources/src/main/res/values/strings.xml`:
- Line 689: Update the msg_no_filter_matches string resource to use
output-neutral wording, such as “No matching entries found.”, so it is
appropriate for both log and build-output panels.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt`:
- Around line 64-80: Update LogFilterBarController.init to invoke
onVisibilityChanged with binding.root.isVisible once after all filter bar
initialization and listeners are configured, ensuring the initial visible state
is propagated without changing toggle() or hide() behavior.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 65-69: Extract the duplicated no-match tracking and filter-active
logic into a shared helper, parameterized by the filter-source predicate and the
relevant filter-bar visibility. Update
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt
lines 65-69 to use it with buildOutputViewModel.filterText.value.isNotEmpty(),
and app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt
lines 148-152 to use it with viewModel.filter.value != LogFilter.NONE; both
sites should reuse the shared noMatchTracker and isFilterActive behavior.
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.kt`:
- Around line 49-54: Document the public setEmptyState function with KDoc,
explicitly defining isEmpty as the layout state and isSourceEmpty as the
source-data state. Describe both parameters so callers pass the values without
confusing or reversing their meanings.
In
`@app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt`:
- Around line 20-22: The new tests use JUnit 4 assertions instead of the
required JUnit Jupiter and Truth APIs. In
app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt
lines 20-22, replace the assertion imports with JUnit Jupiter and Truth imports;
in app/src/test/java/com/itsaky/androidide/logs/LogBufferTest.kt line 23,
replace the added JUnit 4 assertion import; and in lines 98-108, migrate the new
test and enclosing class as needed to JUnit Jupiter and Truth while preserving
its assertions and behavior.
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Around line 97-101: The new public method is missing documentation and unit
coverage for its no-match contract. Add KDoc to
IDEEditorSearcher.isSearchCompleteWithNoMatches() describing that it returns
true only when a query exists, the search result is valid, and the result set is
empty; add tests covering no query, invalid results, empty results, and
non-empty results.
- Around line 35-40: Update IDEEditorSearcher.getEditor() to catch only the
reflection and cast failures it can translate: NoSuchFieldException,
IllegalAccessException, IllegalArgumentException, and ClassCastException. Keep
the existing RuntimeException wrapping for those failures, while allowing fatal
Error values and unrelated assertion-style failures to propagate unchanged.
🪄 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 Plus
Run ID: 9197f347-cf3d-42e3-a005-ac4f6e2e49ac
📒 Files selected for processing (18)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/FilterNoMatchTracker.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/logs/LogBuffer.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.ktapp/src/test/java/com/itsaky/androidide/fragments/output/FilterNoMatchTrackerTest.ktapp/src/test/java/com/itsaky/androidide/logs/LogBufferTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/LogViewModelTest.ktcommon/src/main/java/com/itsaky/androidide/utils/KeyboardUtils.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorActionsMenu.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kteditor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.ktresources/src/main/res/values/strings.xml
🛑 Comments failed to post (3)
app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt (1)
241-248: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep
shareJobownership when a share completes.Line 245 clears
shareJobwithout confirming that this coroutine still owns it. A cancelled job can finish after reattachment and clear a newer share job. Line 248 then enables share and clear actions during the newer share.Only update the shared job state when
shareJobis the current coroutine job.Proposed fix
} finally { - if (isAttachedToWindow) { + val completedJob = coroutineContext[Job] + if (isAttachedToWindow && shareJob === completedJob) { shareJob = null val current = pagerAdapter.getFragmentAtIndex<Fragment>(binding.tabs.selectedTabPosition) val isCurrentEmpty = (current as? EmptyStateFragment<*>)?.isSourceEmpty == true updateActionButtonsEnabledState(isSourceEmpty = isCurrentEmpty) }📝 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.} finally { val completedJob = coroutineContext[Job] if (isAttachedToWindow && shareJob === completedJob) { // The share job is still active while its own finally runs, so it must be // cleared first or updateActionButtonsEnabledState would keep the share and // clear actions disabled. Also re-read the current tab: it may have changed // since the share started. shareJob = null val current = pagerAdapter.getFragmentAtIndex<Fragment>(binding.tabs.selectedTabPosition) val isCurrentEmpty = (current as? EmptyStateFragment<*>)?.isSourceEmpty == true updateActionButtonsEnabledState(isSourceEmpty = isCurrentEmpty) }🤖 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/ui/EditorBottomSheet.kt` around lines 241 - 248, Update the share completion cleanup around shareJob so it clears the shared state and calls updateActionButtonsEnabledState only when shareJob still references the current coroutine job. Preserve the current-tab re-read and empty-state calculation, but prevent an older cancelled share from clearing or enabling actions for a newer share.editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt (1)
97-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C 8 \ 'isSearchCompleteWithNoMatches|isResultValid|lastResults|PublishSearchResultEvent|currentThread' \ --glob '*.{kt,java}' . artifact="$(find "$HOME/.gradle/caches" -type f \( -iname '*sora*.aar' -o -iname '*sora*.jar' \) -print -quit)" if [[ -z "$artifact" ]]; then echo "No resolved Sora artifact found" >&2 exit 2 fi tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT if [[ "$artifact" == *.aar ]]; then unzip -p "$artifact" classes.jar > "$tmp/classes.jar" else cp "$artifact" "$tmp/classes.jar" fi javap -classpath "$tmp/classes.jar" -c -p io.github.rosemoe.sora.widget.EditorSearcher | rg -n -C 12 'isResultValid|lastResults|currentThread|postInLifecycle'Repository: appdevforall/CodeOnTheGo
Length of output: 50381
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== candidate files ==" git ls-files | rg 'IDEEditorSearcher\.kt|EditorSearcher\.java|EditorSearchLayout\.kt' || true echo "== IDEEditorSearcher.kt ==" fd -a IDEEditorSearcher.kt . | while read -r f; do echo "--- ${f#"$PWD/"} ---" wc -l "$f" sed -n '1,220p' "$f" done echo "== relevant local searches ==" rg -n -C 6 \ 'isSearchCompleteWithNoMatches|isResultValid|hasQuery|lastResults|PublishSearchResultEvent|currentThread|postInLifecycle|onSearchResult|gotoPrevious|gotoNext' \ --glob '*.{kt,java}' \ editor termux 2>/dev/null || true echo "== Gradle deps mentioning sora/editor artifacts ==" rg -n 'sora|IDEEditorSearcher|ros.*earch|EditorSearcher' \ gradle libs.versions.toml \ --glob '*.{toml,kts,gradle,properties}' || trueRepository: appdevforall/CodeOnTheGo
Length of output: 19937
🏁 Script executed:
#!/bin/bash set -euo pipefail tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT echo "== Gradle versions/dependencies ==" cat gradle/libs.versions.toml | sed -n '150,168p' echo "== Search sora artifacts in Gradle cache ==" find "$HOME/.gradle/caches" \ \( -path '*transforms*' -o -path '*modules-2*' -o -path '*artifacts*' \) \ -type f \( -iname 'io.github.Rosemoe.sora-editor*.jar' -o -iname 'io.github.Rosemoe.sora-editor*.aar' -o -iname 'sora-editor-*.jar' -o -iname 'sora-editor-*.aar' \ -o -iname '*EditorSearcher*.jar' -o -iname '*EditorSearcher*.aar' \) \ 2>/dev/null | head -50 for artifact in \ $(find "$HOME/.gradle/caches/modules-2/files-2.1/io.github.Rosemoe.sora-editor" -type f 2>/dev/null | head -20) do echo "--- artifact $artifact ---" case "$artifact" in *.aar) unzip -l "$artifact" | rg 'classes\.jar|EditorSearcher\.class|resources\.arsc|androidmanifest\.xml' || true unzip -p "$artifact" classes.jar > "$tmp/classes.jar" ;; *.jar) cp "$artifact" "$tmp/classes.jar" unzip -l "$artifact" | rg 'EditorSearcher\.class|resources\.properties|resources\.csv' || true ;; esac echo "--- javap EditorSearcher relevant methods ---" javap -classpath "$tmp/classes.jar" -p io.github.rosemoe.sora.widget.EditorSearcher \ | rg -n -C 3 'EditorSearcher|hasQuery|isResultValid|lastResults|currentThread|postInLifecycle|gotoPrevious|gotoNext|search' || true echo "--- bytecode methods around relevant names ---" javap -classpath "$tmp/classes.jar" -c -p io.github.rosemoe.sora.widget.EditorSearcher \ | rg -n -C 16 'isResultValid|lastResults|currentThread|postInLifecycle|onSearchResult|PublishSearchResultEvent|gotoPrevious|gotoNext' || true doneRepository: appdevforall/CodeOnTheGo
Length of output: 1593
Read the published search results instead of
isResultValid().
isSearchCompleteWithNoMatches()returnsfalsewhile the published match list has not arrived, then it can uselastResultsthat belongs to a previous search if the query changes or the worker is cancelled before the lifecycle callback runs. Track a query/result generation id, or consume the result publication callback before readinglastResults, so the no-match message applies only to the current query.🤖 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/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` around lines 97 - 101, Update IDEEditorSearcher.isSearchCompleteWithNoMatches() to rely on the current query’s published search result rather than isResultValid() and potentially stale lastResults. Track or validate a query/result generation through the result publication lifecycle, and only report no matches after the current query’s result has been published; return false while pending, cancelled, or associated with an older query.Source: Learnings
resources/src/main/res/values/strings.xml (1)
689-689: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the no-match message output-neutral.
BuildOutputFragmentalso usesmsg_no_filter_matches. The build-output panel will display “No matching log entries found.” Change this text to “No matching entries found.” or add a separate build-output resource.🤖 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 `@resources/src/main/res/values/strings.xml` at line 689, Update the msg_no_filter_matches string resource to use output-neutral wording, such as “No matching entries found.”, so it is appropriate for both log and build-output panels.
1 participant
Disable text actions (Select, all, Copy, Help) when highlighting search matches. Implemented for file searches (Find in file) and log searches.