ADFA-4436 Add a generic editor decoration provider extension point#1448
Merged
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 Walkthrough
WalkthroughAdds an editor-decoration plugin API and registry, bridges enabled decoration providers into the app on startup and night-mode changes, refreshes that bridge after plugin load, and applies decoration spans during tree-sitter span generation. ChangesEditor decoration plugin flow
Sequence Diagram(s)sequenceDiagram
participant CredentialProtectedApplicationLoader
participant PluginManagerViewModel
participant EditorDecorationBridge
participant PluginManager
participant EditorDecorationRegistry
participant EventBus
CredentialProtectedApplicationLoader->>EditorDecorationBridge: init()
EditorDecorationBridge->>PluginManager: getEnabledEditorDecorationProviders()
EditorDecorationBridge->>EditorDecorationRegistry: update(providers)
EditorDecorationBridge->>EventBus: post(ColorSchemeInvalidatedEvent)
CredentialProtectedApplicationLoader->>EditorDecorationBridge: refresh()
PluginManagerViewModel->>EditorDecorationBridge: refresh()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/app/CredentialProtectedApplicationLoader.kt (1)
329-337: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRegister the bracket bridge even if
loadPlugins()fails.
RainbowBracketBridge.init()sits betweenloadPlugins()and the success log inside thetry. IfloadPlugins()throws,init()is skipped, so the day/night theme listener is never registered and the registry is never seeded — bracket coloring won't react to theme flips for the rest of the session.init()is independent of plugin-load success (a null registry simply means "disabled"), so move it where it always runs.🔧 Proposed fix
GlobalScope.launch { try { pluginManager?.loadPlugins() - com.itsaky.androidide.utils.RainbowBracketBridge.init() logger.info("Plugin system initialized successfully") } catch (e: Exception) { logger.error("Failed to load plugins", e) + } finally { + com.itsaky.androidide.utils.RainbowBracketBridge.init() } }🤖 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/app/CredentialProtectedApplicationLoader.kt` around lines 329 - 337, The bracket bridge setup in GlobalScope.launch is currently inside the same try block as pluginManager?.loadPlugins(), so RainbowBracketBridge.init() is skipped whenever plugin loading throws. Move RainbowBracketBridge.init() out of that failure-prone path so it always runs regardless of loadPlugins() outcome, while keeping the existing success/error logging around the plugin load in CredentialProtectedApplicationLoader.
🤖 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
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 297-333: The bracket depth scan in bracketStateAtLineStart is
rescanning from the start of content on every cache miss, which makes large-file
edits and scrolling quadratic. Update the bracketStateCache used by
bracketStateAtLineStart to support finding the nearest prior cached line-start
state (for example via a floor lookup) and seed the scan from that state instead
of always starting at 0. Keep the existing depth/inBlock parsing logic, but
initialize it from the closest cached prefix so the scan only processes the
remaining suffix.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt`:
- Around line 329-337: The bracket bridge setup in GlobalScope.launch is
currently inside the same try block as pluginManager?.loadPlugins(), so
RainbowBracketBridge.init() is skipped whenever plugin loading throws. Move
RainbowBracketBridge.init() out of that failure-prone path so it always runs
regardless of loadPlugins() outcome, while keeping the existing success/error
logging around the plugin load in CredentialProtectedApplicationLoader.
🪄 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: eb7ff7ad-5c92-4874-86ce-ce26158384d6
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/utils/RainbowBracketBridge.ktapp/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.ktcommon/src/main/java/com/itsaky/androidide/syntax/brackets/RainbowBracketRegistry.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/BracketColorExtension.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
Add an additive, feature-agnostic way for plugins to contribute foreground colors to editor text, layered on top of the normal syntax highlighting. - plugin-api: new EditorDecorationProvider with a single method, decorate(text, start, end, isDark) -> List<DecorationSpan>. The provider owns all of its own logic and returns colored ranges; because the only output is a list of spans, it can never replace or suppress highlighting. - plugin-manager: getEnabledEditorDecorationProviders() collector. - common: EditorDecorationRegistry holds the active providers + current theme, read by the editor pipeline and populated by the app. Keeps the editor module free of any plugin-manager dependency (common now depends on the leaf plugin-api module). - editor-treesitter: after building a region's base spans, LineSpansGenerator calls each provider and merges the returned spans as foreground-only overrides, preserving base styles and the non-overlap invariant. The IDE is entirely feature-agnostic here. - app: EditorDecorationBridge registers the enabled providers and current theme into the registry and posts ColorSchemeInvalidatedEvent to repaint; a ComponentCallbacks listener flips decorations on day/night changes live. The dead EditorExtension.provideSyntaxHighlighting() hook (no call sites, replaces highlighting, no RGB/depth) is left untouched. A companion rainbow-brackets plugin (separate repo) implements this provider end to end; device-verified: depth-cycled bracket colors in Java, string and comment exclusion, day/night palettes, live theme toggle, revert on disable.
84293ea to
3d60906
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt (1)
57-80: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNon-atomic
registeredguard contradicts the "safe to call more than once" doc.The
if (!registered) { registered = true; ... }check-then-set on a@Volatilefield is not atomic, so two concurrentinit()calls could both register aComponentCallbacks. Todayinit()is only invoked once from the plugin-load coroutine, so this is not currently reachable, but acompareAndSetmakes the guarantee in the KDoc actually hold.♻️ Use AtomicBoolean for the one-time guard
- `@Volatile` - private var registered = false + private val registered = java.util.concurrent.atomic.AtomicBoolean(false)- if (!registered) { - registered = true + if (registered.compareAndSet(false, true)) { try {🤖 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/utils/EditorDecorationBridge.kt` around lines 57 - 80, The one-time guard in EditorDecorationBridge.init is not atomic, so concurrent calls can register the ComponentCallbacks twice despite the “safe to call more than once” contract. Replace the plain registered flag with an AtomicBoolean and use a compare-and-set style check in init() to ensure only one thread can enter the registration block. Keep the existing refresh() behavior, and update the init()/registered logic so the guard and callback registration remain tied to EditorDecorationBridge.
🤖 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.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt`:
- Around line 57-80: The one-time guard in EditorDecorationBridge.init is not
atomic, so concurrent calls can register the ComponentCallbacks twice despite
the “safe to call more than once” contract. Replace the plain registered flag
with an AtomicBoolean and use a compare-and-set style check in init() to ensure
only one thread can enter the registration block. Keep the existing refresh()
behavior, and update the init()/registered logic so the guard and callback
registration remain tied to EditorDecorationBridge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7ce9568-5027-40d5-8389-507007f5bbfb
📒 Files selected for processing (8)
app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.ktapp/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.ktcommon/build.gradle.ktscommon/src/main/java/com/itsaky/androidide/syntax/decoration/EditorDecorationRegistry.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/EditorDecorationProvider.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
jatezzz
requested changes
Jun 25, 2026
- Replace fully-qualified EditorDecorationBridge / EditorDecorationProvider references with imports across the app and plugin-manager call sites. - Invert the registration guard in EditorDecorationBridge.init() to an early return, dropping a level of nesting while preserving behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt (1)
78-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid catching
Throwablein these paths.This also swallows unrecoverable failures (
OutOfMemoryError, linkage errors, etc.) and can leave decoration state half-initialized or stale. Catch the specific exception you expect here, or at least narrow this toExceptionso fatal errors still surface. Based on learnings, prefer narrow exception handling in Kotlin instead of broad catch-alls.Also applies to: 98-99
🤖 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/utils/EditorDecorationBridge.kt` around lines 78 - 79, The try/catch blocks in EditorDecorationBridge’s listener registration path are too broad because they catch Throwable and can hide fatal JVM errors; narrow them to the expected failure type or at minimum Exception so unrecoverable errors still propagate. Update both affected catch blocks in EditorDecorationBridge to use specific exception handling around the editor decoration theme listener registration logic while keeping the existing error logging.Source: Learnings
🤖 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/utils/EditorDecorationBridge.kt`:
- Around line 59-80: The one-time registration guard in
EditorDecorationBridge.init is being marked as registered before
registerComponentCallbacks succeeds, and the check/set is not atomic. Update the
init flow so the registered state is only set after successful callback
registration, and make the guard atomic using synchronization or a CAS-style
approach; if registration fails in the catch path, ensure the state is not left
enabled so future init calls can retry.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt`:
- Around line 78-79: The try/catch blocks in EditorDecorationBridge’s listener
registration path are too broad because they catch Throwable and can hide fatal
JVM errors; narrow them to the expected failure type or at minimum Exception so
unrecoverable errors still propagate. Update both affected catch blocks in
EditorDecorationBridge to use specific exception handling around the editor
decoration theme listener registration logic while keeping the existing error
logging.
🪄 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: 36b5c820-02dd-4e56-a733-0781fe8721dc
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.ktapp/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt
- app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt
Claim the one-time listener registration with AtomicBoolean.compareAndSet so concurrent init() callers can't double-register, and roll the flag back on failure so a later init() can retry instead of being permanently stuck.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt (1)
76-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid catching
Throwablein the bridge paths.Lines 76-79 and 98-99 currently swallow fatal JVM errors as well as ordinary failures, which can leave the app limping along after conditions like
OutOfMemoryErrororLinkageError. Please narrow this to the concrete exception you expect here, or at leastException, and let fatal errors fail fast. Based on learnings, similar Kotlin crash handling in this project should prefer narrow exception catches over catch-all handlers.Also applies to: 98-99
🤖 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/utils/EditorDecorationBridge.kt` around lines 76 - 79, Avoid catching Throwable in EditorDecorationBridge’s bridge paths; narrow the handler in the listener registration and cleanup code to the specific expected exception type, or at most Exception, so fatal JVM errors are not swallowed. Update the catch blocks around the editor decoration theme listener registration and the related bridge cleanup path to keep the existing log/error handling but let unrecoverable errors fail fast.Source: Learnings
🤖 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.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt`:
- Around line 76-79: Avoid catching Throwable in EditorDecorationBridge’s bridge
paths; narrow the handler in the listener registration and cleanup code to the
specific expected exception type, or at most Exception, so fatal JVM errors are
not swallowed. Update the catch blocks around the editor decoration theme
listener registration and the related bridge cleanup path to keep the existing
log/error handling but let unrecoverable errors fail fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52663d7c-7dbe-42fe-bacc-07799e2ada77
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/utils/EditorDecorationBridge.kt
jatezzz
approved these changes
Jun 26, 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.
Summary
Adds a generic, additive way for plugins to contribute foreground colors to editor text, layered on top of normal syntax highlighting — never replacing it. The IDE is feature-agnostic: it knows nothing about what a provider decorates (brackets, indent guides, markers, …); it just merges the spans a provider returns.
Motivation: the only pre-existing editor-coloring hook,
EditorExtension.provideSyntaxHighlighting(), is dead code (zero call sites) and semantically unsuited — it returns a fixed semantic enum with no RGB/depth and would replace all highlighting. It is left untouched here.API
The provider receives the full document content and a region, and returns additive, foreground-only color spans. All feature logic lives in the plugin.
Changes
EditorDecorationProvider+DecorationSpan.getEnabledEditorDecorationProviders()collector.EditorDecorationRegistryholds the active providers + current theme; read by the editor pipeline, populated by the app. Keeps the editor module free of any plugin-manager dependency (commonnow depends on the leafplugin-apimodule).LineSpansGeneratorcalls each provider and merges the returned spans as foreground-only overrides, preserving base styles and the strictly-ascending non-overlap invariant. ~30 generic lines; no feature concepts.EditorDecorationBridgeregisters enabled providers + current theme into the registry and postsColorSchemeInvalidatedEventto repaint; aComponentCallbackslistener flips decorations on day/night changes live.Dependency direction stays clean:
app → plugin-manager,app → common,editor → common → plugin-api; the editor never depends on the plugin manager.Verification
Device-verified on an emulator with a companion rainbow-brackets plugin (separate repo) that implements
EditorDecorationProvider:(),[],{}in a Java file (provider scans text itself, so it's language-agnostic).Notes
This is the IDE-side generic API + merge. The rainbow plugin (which owns all bracket/depth/palette logic) lives in the plugin-examples repo and compiles against the refreshed
plugin-api.jar.(Reworked from an earlier rainbow-specific
BracketColorExtensionafter review feedback that feature logic shouldn't live in the IDE.)