feat(lsp): implement go to definition/references & rename symbol #885

KonerDev · 2025-10-18T13:05:24Z

This PR adds IDE-like features such as Go to definition, Go to references or Rename symbol and fixes several issues.

Changes

Add code editor actions as described in LSP issue board: Current limitations and issues #863
- Add ability to register text action items in sora-editor
- Add go to definition
- Add go to references
  - Add definition & references dialog if more than one was found
- Add rename symbols
  - Add rename dialog
Fix that theme does not apply to signature popup as described in LSP issue board: Current limitations and issues #863
Fix editor not loading huge files (using CompletableDeferred<Unit>())
Remove unused strings and fix dialog message (Use Discard instead of undescriptive Close)

Current limitations & issues

DocumentChanges is not yet supported in textDocument/rename (Shouldn't be important for renaming symbols)
Rename symbol action only supports currently opened file and does not allow renaming across different files (see caution below for reason)
Undoing rename symbol action does not undo the whole action in one step (see Undo after applyEdits should revert all edits in one step Rosemoe/sora-editor#722)

Caution

In the BaseLspConnector.kt we currently create a new LspProject for every single file. I'm pretty sure though that multiple files should share the same LspProject because otherwise the language server might not be able to handle e.g. rename symbol actions across different files. This is just a guess. Reference:
https://github.com/Xed-Editor/Xed-Editor/blob/3f7dbb4a909d9877e4c299da50b12c847565f941/core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt#L58C13-L58C64

…tions

github-actions · 2025-10-18T13:05:34Z

PR Summary

This PR introduces core IDE-like features leveraging Language Server Protocol (LSP), including "Go to definition", "Go to references", and "Rename symbol". It enhances the KarbonEditor with the ability to register text action items and fixes issues such as theme application to signature popups and loading large files. New dialogs for displaying code findings and facilitating symbol renaming have been added, alongside new icons and updated string resources.

Changes

File	Summary
`core/main/src/main/assets/supported_locales.json`	Added "ota" to the list of supported locales in the `supported_locales.json` file.
`core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt`	Implemented new methods to support LSP features like "Go to definition", "Go to references", and "Rename symbol". This includes checking server capabilities and making corresponding LSP requests. Unused imports were removed.
`core/main/src/main/java/com/rk/libcommons/editor/KarbonEditor.kt`	Updated `setThemeColors` to correctly apply the user theme to the signature popup. Added `registerTextAction` and `unregisterTextAction` methods to enable dynamic registration of text action items within the editor.
`core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt`	Introduced `LspActions.kt` to encapsulate LSP-related text actions. It provides utility functions for fixing LSP URIs, generating code snippets, navigating to code locations, and defines `TextActionItem`s for "Go to definition", "Go to references", and "Rename symbol" features.
`core/main/src/main/java/com/rk/tabs/EditorTab.kt`	Enhanced `EditorTab` to support new LSP features by adding state variables for findings and rename dialogs. It now integrates `FindingsDialog` and `SingleInputDialog` for user interaction. The editor initialization was improved to handle large files and LSP text actions are registered dynamically.
`core/main/src/main/java/com/rk/xededitor/ui/activities/main/MainContent.kt`	Updated the "ask unsaved changes" dialog to use "Discard" instead of "Close" for clarity when a tab is being removed.
`core/main/src/main/java/com/rk/xededitor/ui/components/EditorSearch.kt`	Removed unused imports related to toast messages and event subscriptions, streamlining the `EditorSearch` component.
`core/main/src/main/java/com/rk/xededitor/ui/components/FileActionDialog.kt`	Removed a debug toast message that displayed the success status after a file rename operation, cleaning up the UI feedback.
`core/main/src/main/java/com/rk/xededitor/ui/components/FindingsDialog.kt`	Introduced `FindingsDialog.kt` to provide a reusable Composable dialog for displaying lists of code items, such as definitions or references. It groups items by filename and allows navigation upon selection.
`core/resources/src/main/res/drawable/edit_note.xml`	New drawable XML file for an "edit note" icon, intended for the rename symbol action within the editor.
`core/resources/src/main/res/drawable/jump_to_element.xml`	New drawable XML file for a "jump to element" icon, intended for the go to definition action within the editor.
`core/resources/src/main/res/drawable/manage_search.xml`	New drawable XML file for a "manage search" icon, intended for the go to references action within the editor.
`core/resources/src/main/res/values/strings.xml`	Removed obsolete strings and added new string resources to support the "Go to definition", "Go to references", and "Rename symbol" LSP features, including descriptions and error messages.

autogenerated by presubmit.ai

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (10)

f723f32: fix(main): register text action buttons and fix huge files not loading
f21d972: feat(main): implement definition, references and rename symbol LSP actions
51be902: feat(main): add dialog for displaying code items
a66acf4: chore(main): remove unused imports
29e1609: fix(main): remove debug toast
026479a: feat(main): allow registration of text action items
b632359: chore(resources): remove unused strings
73230ec: fix(main): replace close with correct discard text
16d2bbd: feat(resources): add icons for LSP actions
80db80f: fix(lsp-support): respect user theme for signature window

Files Processed (13)

core/main/src/main/assets/supported_locales.json (1 hunk)
core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt (3 hunks)
core/main/src/main/java/com/rk/libcommons/editor/KarbonEditor.kt (5 hunks)
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt (1 hunk)
core/main/src/main/java/com/rk/tabs/EditorTab.kt (8 hunks)
core/main/src/main/java/com/rk/xededitor/ui/activities/main/MainContent.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/EditorSearch.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/FileActionDialog.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/FindingsDialog.kt (1 hunk)
core/resources/src/main/res/drawable/edit_note.xml (1 hunk)
core/resources/src/main/res/drawable/jump_to_element.xml (1 hunk)
core/resources/src/main/res/drawable/manage_search.xml (1 hunk)
core/resources/src/main/res/values/strings.xml (2 hunks)

Actionable Comments (2)

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt [136-141]

possible bug: "Address potential NullPointerExceptions in LSP requests."
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [275-275]

possible bug: "Handle nullability when accessing workspace edit changes."

Skipped Comments (5)

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt [135-142]

performance: "Avoid blocking calls on the default dispatcher."
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [68-72]

readability: "Review whitespace trimming in code snippets."
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [124-144]

maintainability: "Refactor duplicated logic for extracting definition details."
core/main/src/main/java/com/rk/tabs/EditorTab.kt [418-435]

possible bug: "Review LSP connection flow to ensure language is always set."
core/main/src/main/java/com/rk/tabs/EditorTab.kt [217-220]

maintainability: "Remove redundant error check for rename input."

github-actions · 2025-10-18T13:06:25Z

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt

+            lspEditor!!.languageServerWrapper.requestManager!!.definition(
+                DefinitionParams(
+                    TextDocumentIdentifier(fileObject!!.getAbsolutePath()),
+                    Position(editor.cursor.leftLine, editor.cursor.leftColumn)
+                )
+            )!!.get(Timeout[Timeouts.EXECUTE_COMMAND].toLong(), TimeUnit.MILLISECONDS)


The use of the !! operator on lspEditor and languageServerWrapper.requestManager assumes these objects are never null after connect is called. While connect is intended to initialize lspEditor, there might be edge cases (e.g., connection failure, disposeAsync being called) where they could be null, leading to a NullPointerException. Consider handling nullability explicitly, perhaps by returning a nullable type or throwing a more specific exception if the LSP connection is not ready.

This method won't be called if there was a language server connection failure.

github-actions · 2025-10-18T13:06:25Z

core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt

+                            return@launch
+                        }
+
+                        val edits = changes[file.toUri().toString()]!!


The line val edits = changes[file.toUri().toString()]!! assumes that changes will always contain an entry for the current file's URI. If changes is empty or the current file's URI is not a key, this will result in a NullPointerException. A safer approach would be to use changes[file.toUri().toString()]?.let { ... } or check for its existence before force-unwrapping.

github-actions

🚨 Pull request needs attention.

Review Summary

Commits Considered (1)

a52b165: fix(main): replace hardcoded string with XML resource

Files Processed (13)

core/main/src/main/assets/supported_locales.json (1 hunk)
core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt (3 hunks)
core/main/src/main/java/com/rk/libcommons/editor/KarbonEditor.kt (5 hunks)
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt (1 hunk)
core/main/src/main/java/com/rk/tabs/EditorTab.kt (8 hunks)
core/main/src/main/java/com/rk/xededitor/ui/activities/main/MainContent.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/EditorSearch.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/FileActionDialog.kt (1 hunk)
core/main/src/main/java/com/rk/xededitor/ui/components/FindingsDialog.kt (1 hunk)
core/resources/src/main/res/drawable/edit_note.xml (1 hunk)
core/resources/src/main/res/drawable/jump_to_element.xml (1 hunk)
core/resources/src/main/res/drawable/manage_search.xml (1 hunk)
core/resources/src/main/res/values/strings.xml (2 hunks)

Actionable Comments (4)

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt [136-141]

possible bug: "Potential NullPointerException in LSP requests"
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [114-114]

possible bug: "Potential NullPointerException in LSP action"
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [269-274]

possible bug: "Potential NullPointerException when applying workspace edits"
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [275-278]

possible bug: "Unsafe null assertion in event emission"

Skipped Comments (3)

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt [141-141]

performance: "Blocking call with timeout in LSP request"
core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt [290-290]

maintainability: "Missing newline at end of file"
core/main/src/main/java/com/rk/tabs/EditorTab.kt [217-218]

maintainability: "Redundant blank check for rename value"

github-actions · 2025-10-18T13:15:19Z

core/main/src/main/java/com/rk/libcommons/editor/BaseLspConnector.kt

+            lspEditor!!.languageServerWrapper.requestManager!!.definition(
+                DefinitionParams(
+                    TextDocumentIdentifier(fileObject!!.getAbsolutePath()),
+                    Position(editor.cursor.leftLine, editor.cursor.leftColumn)
+                )
+            )!!.get(Timeout[Timeouts.EXECUTE_COMMAND].toLong(), TimeUnit.MILLISECONDS)


The use of lspEditor!! and languageServerWrapper.requestManager!! assumes these objects are never null. While connect is intended to initialize lspEditor, there might be edge cases where they could be null, leading to a NullPointerException. Consider handling nullability explicitly, perhaps by returning a nullable type or throwing a more specific exception if the LSP connection is not ready. This applies to all request* methods.

github-actions · 2025-10-18T13:15:19Z

core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt

+    ) { editor ->
+        scope.launch(Dispatchers.Default) {
+            runCatching {
+                val eitherDefinitions = baseLspConnectorProvider()!!.requestDefinition(editor)


The baseLspConnectorProvider()!! call here can lead to a NullPointerException if the provider returns null. Although shouldShow checks for true, the action block itself doesn't re-verify or handle the null case. It's safer to use a nullable check or a let block to ensure baseLspConnector is not null before proceeding.

github-actions · 2025-10-18T13:15:19Z

core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt

+                        if (changes.size > 1) {
+                            toast(strings.rename_symbol_multiple_files)
+                            return@launch
+                        }
+
+                        val edits = changes[file.toUri().toString()]!!


The line val edits = changes[file.toUri().toString()]!! assumes that changes will always contain an entry for the current file's URI. If changes is empty or the current file's URI is not a key, this will result in a NullPointerException. A safer approach would be to use changes[file.toUri().toString()]?.let { ... } or check for its existence before force-unwrapping. The if (changes.size > 1) check is a limitation, but the !! is a direct bug.

github-actions · 2025-10-18T13:15:19Z

core/main/src/main/java/com/rk/libcommons/editor/LspActions.kt

+                        baseLspConnectorProvider()!!.getEventManager()!!.emitAsync(EventType.applyEdits) {
+                            put("edits", edits)
+                            put(editor.text)
+                        }


Similar to other !! usages, baseLspConnectorProvider()!!.getEventManager()!! can cause a NullPointerException if either baseLspConnectorProvider() or getEventManager() returns null. It's important to handle these nullability cases explicitly to prevent crashes.

github-actions

✅ LGTM!

Review Summary

Commits Considered (1)

e3cec89: fix(soraX): soraX changes

Files Processed (1)

soraX (1 hunk)

Actionable Comments (0)

Skipped Comments (0)

This reverts commit e3cec89.

github-actions

✅ LGTM!

Review Summary

Commits Considered (1)

1600eea: Revert "fix(soraX): soraX changes"

This reverts commit e3cec89.

Files Processed (0)

Actionable Comments (0)

Skipped Comments (0)

KonerDev added 10 commits October 17, 2025 18:55

fix(lsp-support): respect user theme for signature window

80db80f

feat(resources): add icons for LSP actions

16d2bbd

fix(main): replace close with correct discard text

73230ec

chore(resources): remove unused strings

b632359

feat(main): allow registration of text action items

026479a

fix(main): remove debug toast

29e1609

chore(main): remove unused imports

a66acf4

feat(main): add dialog for displaying code items

51be902

feat(main): implement definition, references and rename symbol LSP ac…

f21d972

…tions

fix(main): register text action buttons and fix huge files not loading

f723f32

github-actions bot reviewed Oct 18, 2025

View reviewed changes

KonerDev changed the title ~~fix(lsp-support): respect user theme for signature window~~ feat(lsp): implement go to definition/references & rename symbol Oct 18, 2025

fix(main): replace hardcoded string with XML resource

a52b165

github-actions bot reviewed Oct 18, 2025

View reviewed changes

fix(soraX): soraX changes

e3cec89

github-actions bot reviewed Oct 18, 2025

View reviewed changes

KonerDev mentioned this pull request Oct 18, 2025

feat(editor): allow registration of text action items Xed-Editor/soraX#1

Merged

Revert "fix(soraX): soraX changes"

1600eea

This reverts commit e3cec89.

github-actions bot reviewed Oct 18, 2025

View reviewed changes

RohitKushvaha01 merged commit 74b63b6 into Xed-Editor:main Oct 18, 2025

KonerDev deleted the feature/lsp-support branch October 19, 2025 08:44

Uh oh!

feat(lsp): implement go to definition/references & rename symbol #885

feat(lsp): implement go to definition/references & rename symbol #885

Uh oh!

Conversation

KonerDev commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Current limitations & issues

Uh oh!

github-actions bot commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Changes

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

KonerDev Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KonerDev commented Oct 18, 2025 •

edited

Loading

github-actions bot commented Oct 18, 2025 •

edited

Loading