Skip to content

Commit f7a0b92

Browse files
authored
fix: use python code when renaming variables (#7380)
1 parent 7a4da52 commit f7a0b92

File tree

3 files changed

+168
-8
lines changed

3 files changed

+168
-8
lines changed

frontend/src/core/codemirror/lsp/__tests__/notebook-lsp.test.ts

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import * as LSP from "vscode-languageserver-protocol";
1111
import type { CellId } from "@/core/cells/ids";
1212
import { store } from "@/core/state/jotai";
1313
import { topologicalCodesAtom } from "../../copilot/getCodes";
14+
import { languageAdapterState } from "../../language/extension";
15+
import { PythonLanguageAdapter } from "../../language/languages/python";
16+
import { languageMetadataField } from "../../language/metadata";
1417
import { createNotebookLens } from "../lens";
1518
import { NotebookLanguageServerClient } from "../notebook-lsp";
1619
import { CellDocumentUri, type ILanguageServerClient } from "../types";
@@ -372,6 +375,8 @@ describe("NotebookLanguageServerClient", () => {
372375
const mockView1 = new EditorView({
373376
doc: "# this is a comment",
374377
extensions: [
378+
languageAdapterState.init(() => new PythonLanguageAdapter()),
379+
languageMetadataField.init(() => ({})),
375380
languageServerWithClient({
376381
client: mockClient as unknown as LanguageServerClient,
377382
documentUri: CellDocumentUri.of(Cells.cell1),
@@ -384,6 +389,8 @@ describe("NotebookLanguageServerClient", () => {
384389
const mockView2 = new EditorView({
385390
doc: "import math\nimport numpy",
386391
extensions: [
392+
languageAdapterState.init(() => new PythonLanguageAdapter()),
393+
languageMetadataField.init(() => ({})),
387394
languageServerWithClient({
388395
client: mockClient as unknown as LanguageServerClient,
389396
documentUri: CellDocumentUri.of(Cells.cell2),
@@ -396,6 +403,8 @@ describe("NotebookLanguageServerClient", () => {
396403
const mockView3 = new EditorView({
397404
doc: "print(math.sqrt(4))",
398405
extensions: [
406+
languageAdapterState.init(() => new PythonLanguageAdapter()),
407+
languageMetadataField.init(() => ({})),
399408
languageServerWithClient({
400409
client: mockClient as unknown as LanguageServerClient,
401410
documentUri: CellDocumentUri.of(Cells.cell3),
@@ -600,6 +609,153 @@ describe("NotebookLanguageServerClient", () => {
600609
// Should return the original response when edits don't have proper properties
601610
expect(result).toEqual(invalidEditResponse);
602611
});
612+
613+
it("should handle raw strings with markdown content during rename (issue #7377)", async () => {
614+
const props = {
615+
workspaceFolders: null,
616+
capabilities: {
617+
textDocument: {
618+
rename: {
619+
prepareSupport: true,
620+
},
621+
},
622+
},
623+
languageId: "python",
624+
transport: {
625+
sendData: vi.fn(),
626+
subscribe: vi.fn(),
627+
connect: vi.fn(),
628+
transportRequestManager: {
629+
send: vi.fn(),
630+
},
631+
} as any,
632+
};
633+
634+
// Setup mock plugins with editor views
635+
const markdownCell = 'mo.md(r"""\n# Header\n""")';
636+
const variableCell = "a = 'Test'";
637+
638+
const mockView1 = new EditorView({
639+
doc: "import marimo as mo",
640+
extensions: [
641+
languageAdapterState.init(() => new PythonLanguageAdapter()),
642+
languageMetadataField.init(() => ({})),
643+
languageServerWithClient({
644+
client: mockClient as unknown as LanguageServerClient,
645+
documentUri: CellDocumentUri.of(Cells.cell1),
646+
...props,
647+
}),
648+
],
649+
});
650+
651+
const mockView2 = new EditorView({
652+
doc: markdownCell,
653+
extensions: [
654+
languageAdapterState.init(() => new PythonLanguageAdapter()),
655+
languageMetadataField.init(() => ({})),
656+
languageServerWithClient({
657+
client: mockClient as unknown as LanguageServerClient,
658+
documentUri: CellDocumentUri.of(Cells.cell2),
659+
...props,
660+
}),
661+
],
662+
});
663+
664+
const mockView3 = new EditorView({
665+
doc: variableCell,
666+
extensions: [
667+
languageAdapterState.init(() => new PythonLanguageAdapter()),
668+
languageMetadataField.init(() => ({})),
669+
languageServerWithClient({
670+
client: mockClient as unknown as LanguageServerClient,
671+
documentUri: CellDocumentUri.of(Cells.cell3),
672+
...props,
673+
}),
674+
],
675+
});
676+
677+
(notebookClient as any).getNotebookEditors = () => ({
678+
[Cells.cell1]: mockView1,
679+
[Cells.cell2]: mockView2,
680+
[Cells.cell3]: mockView3,
681+
});
682+
683+
// Update the mock to return the correct codes
684+
vi.spyOn(store, "get").mockImplementation((atom) => {
685+
if (atom === topologicalCodesAtom) {
686+
return {
687+
cellIds: [Cells.cell1, Cells.cell2, Cells.cell3],
688+
codes: {
689+
[Cells.cell1]: "import marimo as mo",
690+
[Cells.cell2]: markdownCell,
691+
[Cells.cell3]: variableCell,
692+
},
693+
};
694+
}
695+
return undefined;
696+
});
697+
698+
// Setup rename params - renaming variable 'a' in cell3
699+
const renameParams: LSP.RenameParams = {
700+
textDocument: { uri: CellDocumentUri.of(Cells.cell3) },
701+
position: { line: 0, character: 0 }, // position of 'a'
702+
newName: "b",
703+
};
704+
705+
// Open a document first to set up the lens
706+
await notebookClient.textDocumentDidOpen({
707+
textDocument: {
708+
uri: CellDocumentUri.of(Cells.cell3),
709+
languageId: "python",
710+
version: 1,
711+
text: variableCell,
712+
},
713+
});
714+
715+
// Mock the response from the language server
716+
// The merged document includes all cells (5 lines total):
717+
// Line 0: "import marimo as mo"
718+
// Line 1: "mo.md(r\"\"\""
719+
// Line 2: "# Header"
720+
// Line 3: "\"\"\")"
721+
// Line 4: "a = 'Test'"
722+
// When renaming variable 'a' to 'b', the entire document is returned
723+
const mockRenameResponse: LSP.WorkspaceEdit = {
724+
documentChanges: [
725+
{
726+
textDocument: {
727+
uri: "file:///__marimo_notebook__.py",
728+
version: 1,
729+
},
730+
edits: [
731+
{
732+
range: {
733+
start: { line: 0, character: 0 },
734+
end: { line: 4, character: 10 },
735+
},
736+
// The renamed text should preserve all cells including markdown
737+
newText:
738+
'import marimo as mo\nmo.md(r"""\n# Header\n""")\nb = \'Test\'',
739+
},
740+
],
741+
},
742+
],
743+
};
744+
745+
mockClient.textDocumentRename = vi
746+
.fn()
747+
.mockResolvedValue(mockRenameResponse);
748+
749+
// Call rename
750+
await notebookClient.textDocumentRename(renameParams);
751+
752+
// Verify that the markdown cell was NOT corrupted and remains unchanged
753+
expect(mockView2.state.doc.toString()).toBe(markdownCell);
754+
// Verify that the variable cell was renamed
755+
expect(mockView3.state.doc.toString()).toBe("b = 'Test'");
756+
// Verify that the import cell was not changed
757+
expect(mockView1.state.doc.toString()).toBe("import marimo as mo");
758+
});
603759
});
604760

605761
describe("diagnostics handling", () => {

frontend/src/core/codemirror/lsp/lens.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ export function createNotebookLens(
8181
const oldLines = mergedText.split("\n");
8282

8383
if (newLines.length !== oldLines.length) {
84+
Logger.warn(
85+
"[lsp] cannot apply rename with new lines",
86+
newLines,
87+
oldLines,
88+
);
8489
throw new Error("Cannot apply rename when there are new lines");
8590
}
8691

frontend/src/core/codemirror/lsp/notebook-lsp.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import { Logger } from "@/utils/Logger";
1010
import { LRUCache } from "@/utils/lru";
1111
import { Objects } from "@/utils/objects";
1212
import { topologicalCodesAtom } from "../copilot/getCodes";
13+
import {
14+
getEditorCodeAsPython,
15+
updateEditorCodeFromPython,
16+
} from "../language/utils";
1317
import { createNotebookLens, type NotebookLens } from "./lens";
1418
import {
1519
CellDocumentUri,
@@ -442,14 +446,9 @@ export class NotebookLanguageServerClient implements ILanguageServerClient {
442446
}
443447

444448
// Only update if it has changed
445-
if (ev.state.doc.toString() !== newCode) {
446-
ev.dispatch({
447-
changes: {
448-
from: 0,
449-
to: ev.state.doc.length,
450-
insert: newCode,
451-
},
452-
});
449+
const code = getEditorCodeAsPython(ev);
450+
if (code !== newCode) {
451+
updateEditorCodeFromPython(ev, newCode);
453452
}
454453
}
455454

0 commit comments

Comments
 (0)