diff --git a/CHANGES_CURRENT.md b/CHANGES_CURRENT.md index fda954eaa9..aa26117171 100644 --- a/CHANGES_CURRENT.md +++ b/CHANGES_CURRENT.md @@ -36,6 +36,7 @@ - #3226 - Keybindings: Fix issue opening keybindings.json from the command palette - #3227 - Configuration: Allow string numbers as font sizes - #3230 - Font: Treat "FiraCode-Regular.ttf" as default font +- #3233 - Formatting: Fix buffer de-sync when applying formatting edits (fixes #2196, #2820) ### Performance diff --git a/development_extensions/oni-dev-extension/extension.js b/development_extensions/oni-dev-extension/extension.js index 4a784a9e95..cc571dd7e5 100644 --- a/development_extensions/oni-dev-extension/extension.js +++ b/development_extensions/oni-dev-extension/extension.js @@ -35,6 +35,21 @@ function activate(context) { }), ) + cleanup( + vscode.commands.registerCommand("developer.oni.logBufferUpdates", () => { + vscode.window.showInformationMessage("Logging buffer updates!"); + vscode.workspace.onDidChangeTextDocument((e) => { + console.log({ + type: "workspace.onDidChangeTextDocument", + filename: e.document.fileName, + version: e.document.version, + contentChanges: e.contentChanges, + fullText: e.document.getText(), + }); + }); + }), + ) + cleanup( vscode.commands.registerCommand("developer.oni.tryOpenDocument", () => { vscode.workspace.openTextDocument(vscode.Uri.file("package.json")) diff --git a/development_extensions/oni-dev-extension/package.json b/development_extensions/oni-dev-extension/package.json index 939df280a0..ed33b84669 100644 --- a/development_extensions/oni-dev-extension/package.json +++ b/development_extensions/oni-dev-extension/package.json @@ -40,6 +40,10 @@ "command": "developer.oni.tryOpenDocument", "title": "Onivim - Developer: Try Open Document (package.json)" }, + { + "command": "developer.oni.logBufferUpdates", + "title": "Onivim - Developer: Log Buffer Updates" + }, { "command": "developer.oni.showWorkspaceRootPath", "title": "Onivim - Developer: Show Workspace Root Path" diff --git a/integration_test/ExtHostBufferUpdatesTest.re b/integration_test/ExtHostBufferUpdatesTest.re index 96ae3b6689..eb22f259de 100644 --- a/integration_test/ExtHostBufferUpdatesTest.re +++ b/integration_test/ExtHostBufferUpdatesTest.re @@ -87,6 +87,7 @@ runTest(~name="ExtHostBufferUpdates", ({input, dispatch, wait, key, _}) => { // Finally, modify a single line input("gg"); input("Iabc"); + key(EditorInput.Key.Escape); TS.validateTextIsSynchronized( ~expectedText=Some("abca|b|c|"), @@ -94,4 +95,28 @@ runTest(~name="ExtHostBufferUpdates", ({input, dispatch, wait, key, _}) => { dispatch, wait, ); + + // Insert multiple lines, and then undo + // This was a case discovered while investigating #2196 + input("gg"); + input("O"); + key(EditorInput.Key.Return); + key(EditorInput.Key.Return); + key(EditorInput.Key.Escape); + + TS.validateTextIsSynchronized( + ~expectedText=Some("|||abca|b|c|"), + ~description="after inserting multiple lines", + dispatch, + wait, + ); + + // Undo the change - we also had bugs here! + input("u"); + TS.validateTextIsSynchronized( + ~expectedText=Some("abca|b|c|"), + ~description="after inserting multiple lines", + dispatch, + wait, + ); }); diff --git a/src/CLI/Oni_CLI.re b/src/CLI/Oni_CLI.re index c729277360..9a5de1fee7 100644 --- a/src/CLI/Oni_CLI.re +++ b/src/CLI/Oni_CLI.re @@ -20,6 +20,7 @@ type t = { shouldLoadConfiguration: bool, shouldSyntaxHighlight: bool, attachToForeground: bool, + logExthost: bool, logLevel: option(Timber.Level.t), logFile: option(string), logFilter: option(string), @@ -93,6 +94,7 @@ let parse = (~getenv: string => option(string), args) => { let attachToForeground = ref(false); let logLevel = ref(None); let isSilent = ref(false); + let logExthost = ref(false); let logFile = ref(None); let logFilter = ref(None); let logColorsEnabled = ref(None); @@ -142,6 +144,7 @@ let parse = (~getenv: string => option(string), args) => { ("-v", setEffect(PrintVersion), ""), ("--nofork", Unit(setAttached), ""), ("--debug", Unit(() => logLevel := Some(Timber.Level.debug)), ""), + ("--debug-exthost", Unit(() => logExthost := true), ""), ("--trace", Unit(() => logLevel := Some(Timber.Level.trace)), ""), ("--quiet", Unit(() => logLevel := Some(Timber.Level.warn)), ""), ( @@ -290,6 +293,7 @@ let parse = (~getenv: string => option(string), args) => { shouldSyntaxHighlight: shouldSyntaxHighlight^, attachToForeground: attachToForeground^, logLevel: logLevel^, + logExthost: logExthost^, logFile: logFile^, logFilter: logFilter^, logColorsEnabled: logColorsEnabled^, diff --git a/src/CLI/Oni_CLI.rei b/src/CLI/Oni_CLI.rei index 3ab8517999..d0d71ca0a9 100644 --- a/src/CLI/Oni_CLI.rei +++ b/src/CLI/Oni_CLI.rei @@ -10,6 +10,7 @@ type t = { shouldLoadConfiguration: bool, shouldSyntaxHighlight: bool, attachToForeground: bool, + logExthost: bool, logLevel: option(Timber.Level.t), logFile: option(string), logFilter: option(string), diff --git a/src/Core/Buffer.re b/src/Core/Buffer.re index 0d0e6103db..d88915a370 100644 --- a/src/Core/Buffer.re +++ b/src/Core/Buffer.re @@ -337,3 +337,20 @@ let setFont = (font, buf) => { let getSaveTick = ({saveTick, _}) => saveTick; let incrementSaveTick = buffer => {...buffer, saveTick: buffer.saveTick + 1}; + +let toDebugString = buf => { + let lines = + buf + |> getLines + |> Array.to_list + |> List.mapi((idx, str) => + "Line " ++ string_of_int(idx) ++ ": |" ++ str ++ "|" + ) + |> String.concat("\n"); + Printf.sprintf( + "Buffer %d (version %d):\n---\n%s\n---\n", + getId(buf), + getVersion(buf), + lines, + ); +}; diff --git a/src/Core/Buffer.rei b/src/Core/Buffer.rei index 3e0c637bda..22e9d3da2e 100644 --- a/src/Core/Buffer.rei +++ b/src/Core/Buffer.rei @@ -88,3 +88,5 @@ let setFont: (Font.t, t) => t; let getSaveTick: t => int; let incrementSaveTick: t => t; + +let toDebugString: t => string; diff --git a/src/Core/BufferUpdate.re b/src/Core/BufferUpdate.re index 5a972a65a0..3ffa73511a 100644 --- a/src/Core/BufferUpdate.re +++ b/src/Core/BufferUpdate.re @@ -20,3 +20,19 @@ let create = version, isFull, }; + +let toDebugString = ({isFull, version, startLine, endLine, lines, _}) => { + let lineStr = + lines + |> Array.to_list + |> List.mapi((idx, str) => Printf.sprintf("Line %d: |%s|", idx, str)) + |> String.concat("\n"); + Printf.sprintf( + "Core.BufferUpdate - version %d (full: %b) - startLine: %d endLine:%d\nLines:\n---\n%s\n---\n\n", + version, + isFull, + startLine |> LineNumber.toZeroBased, + endLine |> LineNumber.toZeroBased, + lineStr, + ); +}; diff --git a/src/Core/BufferUpdate.rei b/src/Core/BufferUpdate.rei index 04f73fd7b1..982dc49092 100644 --- a/src/Core/BufferUpdate.rei +++ b/src/Core/BufferUpdate.rei @@ -22,3 +22,5 @@ let create: unit ) => t; + +let toDebugString: t => string; diff --git a/src/Exthost/ModelContentChange.re b/src/Exthost/ModelContentChange.re index 13569b5ab1..411c648f7e 100644 --- a/src/Exthost/ModelContentChange.re +++ b/src/Exthost/ModelContentChange.re @@ -17,17 +17,16 @@ let create = (~rangeLength: int, ~range: CharacterRange.t, ~text: string, ()) => }; let joinLines = (separator: string, lines: list(string)) => { - String.concat(separator, lines); + let joined = String.concat(separator, lines); + + if (lines != []) { + joined ++ separator; + } else { + joined; + }; }; let getRangeFromEdit = (bu: BufferUpdate.t) => { - let newLines = Array.length(bu.lines); - let isInsert = - EditorCoreTypes.( - newLines >= LineNumber.toZeroBased(bu.endLine) - - LineNumber.toZeroBased(bu.startLine) - ); - let startLine = EditorCoreTypes.LineNumber.toZeroBased(bu.startLine); let endLine = EditorCoreTypes.LineNumber.toZeroBased(bu.endLine) |> max(startLine); @@ -48,7 +47,7 @@ let getRangeFromEdit = (bu: BufferUpdate.t) => { } ); - (isInsert, range); + range; }; let getRangeLengthFromEdit = @@ -72,11 +71,9 @@ let getRangeLengthFromEdit = let ofBufferUpdate = (~previousBuffer, bu: Oni_Core.BufferUpdate.t, eol: Eol.t) => { - let (isInsert, range) = getRangeFromEdit(bu); + let range = getRangeFromEdit(bu); let text = joinLines(Eol.toString(eol), bu.lines |> Array.to_list); let rangeLength = getRangeLengthFromEdit(~previousBuffer, ~eol, bu); - let text = isInsert ? text ++ Eol.toString(eol) : text; - {range: OneBasedRange.ofRange(range), text, rangeLength}; }; diff --git a/src/Feature/Buffers/Feature_Buffers.re b/src/Feature/Buffers/Feature_Buffers.re index cdc97c9990..8002c4bb34 100644 --- a/src/Feature/Buffers/Feature_Buffers.re +++ b/src/Feature/Buffers/Feature_Buffers.re @@ -384,6 +384,7 @@ let update = (~activeBufferId, ~config, msg: msg, model: model) => { } else { newBuffer; }; + let markerUpdate = MarkerUpdate.create(~update, ~original=oldBuffer, ~updated=buffer); ( diff --git a/src/Store/StoreThread.re b/src/Store/StoreThread.re index 52a2fc89a7..6c76f3a7bf 100644 --- a/src/Store/StoreThread.re +++ b/src/Store/StoreThread.re @@ -129,11 +129,14 @@ let start = let initialState = getState(); - let attachStdio = + let attachExthostStdio = Oni_CLI.( { initialState.cli.attachToForeground - && Option.is_some(initialState.cli.logLevel); + && ( + Option.is_some(initialState.cli.logLevel) + || initialState.cli.logExthost + ); } ); @@ -144,7 +147,7 @@ let start = let (extHostClientResult, extHostStream) = ExtensionClient.create( ~initialWorkspace, - ~attachStdio, + ~attachStdio=attachExthostStdio, ~config=getState().config, ~extensions, ~setup, diff --git a/src/bin_launcher/Oni2.re b/src/bin_launcher/Oni2.re index 62be6c7eec..63c73ed5b6 100644 --- a/src/bin_launcher/Oni2.re +++ b/src/bin_launcher/Oni2.re @@ -34,6 +34,11 @@ let spec = " Stay attached to the foreground terminal.", ), ("--debug", passthrough, " Enable debug logging."), + ( + "--debug-exthost", + passthrough, + " Pipe exthost output to stdout/stderr.", + ), ("--trace", passthrough, " Enable trace logging."), ("--quiet", passthrough, " Print only error log messages."), ("--silent", passthrough, " Do not print any logging."), diff --git a/src/reason-libvim/BufferUpdate.re b/src/reason-libvim/BufferUpdate.re index 017444c7f4..41c38bb457 100644 --- a/src/reason-libvim/BufferUpdate.re +++ b/src/reason-libvim/BufferUpdate.re @@ -14,9 +14,11 @@ let show = (v: t) => { v.endLine, v.version, ) - ++ "----" - ++ Array.fold_left((s, prev) => prev ++ s ++ "\n", "", v.lines) - ++ "----"; + ++ "---- " + ++ string_of_int(Array.length(v.lines)) + ++ "\n" + ++ Array.fold_left((s, prev) => prev ++ "\n" ++ s, "", v.lines) + ++ "\n----"; }; let getAllLines = (buffer: Native.buffer) => { diff --git a/test/Cli/CliTest.re b/test/Cli/CliTest.re index 988da5e841..e9c986ba3a 100644 --- a/test/Cli/CliTest.re +++ b/test/Cli/CliTest.re @@ -57,6 +57,11 @@ describe("CLI", ({describe, test, _}) => { }) }); describe("log level", ({test, _}) => { + test("--debug-exthost should set logExthost", ({expect, _}) => { + let (options, _eff) = + Oni_CLI.parse(~getenv=noenv, [|"Oni2_editor", "--debug-exthost"|]); + expect.equal(options.logExthost, true); + }); test("--silent should still require a console", ({expect, _}) => { let (options, _eff) = Oni_CLI.parse(~getenv=noenv, [|"Oni2_editor", "-f", "--silent"|]);