Improve JetBrains session usability#11015
Conversation
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Previously Flagged — Resolved
Other Observations (not in diff)
Files Reviewed (incremental, 11 changed files)
Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 2,301,415 tokens Review guidance: REVIEW.md from base branch |
|
Addressed the actionable scroll review warning:
Verification:
I also reran the tool-expansion scroll assertion group; scroll assertions pass, but the known unrelated IntelliJ editor disposal failure still appears in that test path. The screenshot / screen recording item is a PR process suggestion rather than a code change. |
| val field = ToolField(preview(tool), SessionEditorStyle.current()).also { ed -> | ||
| ed.setDisposedWith(disposable) | ||
| Disposer.register(disposable) { | ||
| ed.getEditor(false)?.let(EditorFactory.getInstance()::releaseEditor) |
There was a problem hiding this comment.
WARNING: Potential double-release of the editor due to LIFO disposal ordering.
Disposer.register(disposable, this) (done internally by setDisposedWith) and Disposer.register(disposable) { releaseEditor } register two children on the same parent. IntelliJ's Disposer disposes children in LIFO order, so the explicit releaseEditor lambda fires before EditorTextField.dispose(). That means:
- Lambda:
getEditor(false)→ non-null →EditorFactory.releaseEditor(editor)called. - Then
EditorTextField.dispose(): callsreleaseEditor(myEditor)a second time on the already-released editor.
To avoid this, register the lambda before setDisposedWith so it ends up later in the LIFO queue (i.e., fires after EditorTextField is already disposed and myEditor is null), or skip the explicit lambda and rely solely on setDisposedWith / EditorTextField.dispose() which already releases the editor internally.
| CodeField(file, opts, text).also { ed -> | ||
| ed.setDisposedWith(disposable) | ||
| Disposer.register(disposable) { | ||
| ed.getEditor(false)?.let(EditorFactory.getInstance()::releaseEditor) |
There was a problem hiding this comment.
WARNING: Same potential double-release as in ToolSupport.kt: the releaseEditor lambda is registered after setDisposedWith, so with LIFO disposal it fires first and releases the editor, then EditorTextField.dispose() (called via setDisposedWith) tries to release the same editor again. Register the lambda before setDisposedWith to ensure it runs after EditorTextField is already cleaned up.
|
reviews will be fixed in the next PR |
Summary
This PR makes JetBrains chat sessions easier to read, scan, and interact with during normal use. It focuses on polishing transcript behavior rather than adding one large standalone feature.
Added Improvements
Verification
Screenshots
Light
Dark