fix(scm/onivim#2857): SCM diff markers not showing in gutter (onivim#…

…2868)

__Issue:__ In upgrading the `git` extension, the diff markers stopped showing up.

__Defect:__ Our implementation relied on using a text-content provider to get the 'original' content to diff against. However, the latest git extension uses a different strategy - it supplies a virtual-file-system for the `git://` scheme. We didn't have an implementation for virtual file systems, so we weren't able to get the original content for diffing in this new implementation.

__Fix:__ Wire up the virtual file systems, and try that preferentially to the text content providers. In addition, we also were not handling reading `buffer` byte data from the extension host, so this changes converts it to a json string, so that we can pick it up in the extension client.

Fixes onivim#2857

CHANGES_CURRENT.md

@@ -11,6 +11,7 @@
 - #2846 - UX: Editor Tabs - horizontal scrolling on trackpad is reversed (thanks @SeitaHigashi!)
 - #2865 - OSX: Fix drag-and-drop on dock icon (fixes #2855)
 - #2867 - Editor: Fix viewport shifting when deleting lines with codelens
+- #2868 - SCM: Diff markers not showing up in gutter (fixes #2857)
 
 ### Performance
 

manual_test/cases.md

@@ -7,26 +7,37 @@
 - Run `:echo $PATH`
 - Validate full shell path is available
 
+__Pass:__
+- [ ] OSX
+
 ## 1.2 Validate launches from dock in OSX (#2659)
 
 - Update .zshrc to have blocking input: (`read var`, `echo $var`)
 - Update .zshrc to have canary entry in `PATH`
 - Run Onivim 2 from dock
 - Validate Onivim 2 launches and PATH is correct
 
+__Pass:__
+- [ ] OSX
+
 # 2. First-Run Experience
 
 Test cases covering launching and using Onivim without any persistence or configuration.
 
-# 2.1 No directory set [OSX|Win|Linux]
+## 2.1 No directory set [OSX|Win|Linux]
 
 - Clear the Onivim 2 configuration folder (`rm -rf ~/.config/oni2`)
 - Launch Onivim
 - Verify explorer shows 'No folder opened'
 - Verify Control+P/Command+P shows the 'Welcome' buffer
 - Verify Control+Shift+P/Command+P shows the command palette
 
-# 2.2 Home directory set [OSX]
+__Pass:__
+- [ ] Win
+- [ ] OSX
+- [ ] Linux
+
+## 2.2 Home directory set [OSX]
 
 This case is related to #2742 - previous builds of Onivim may have persisted
 the startup folder as `~/Documents`. This is problematic because Onivim may
@@ -39,3 +50,19 @@ not have permission to read that folder.
 - Verify explorer shows 'No folder opened'
 - Verify Control+P/Command+P shows only the 'Welcome' buffer
 - Verify Control+Shift+P/Command+Shift+P shows the command palette
+
+__Pass:__
+- [ ] OSX
+
+# 3. SCM
+
+## 3.1 Verify diff markers show in modified git file (#2857)
+
+- Launch Onivim in oni2 directory
+- Modify `README.md`
+- Verify diff markers show 
+
+__Pass:__
+- [ ] Win
+- [ ] OSX
+- [ ] Linux

src/Exthost/Client.re

@@ -167,19 +167,15 @@ let start =
         |> Option.iter(resolver => {
              switch (payload) {
              | Json(json) => Lwt.wakeup(resolver, json)
+
              | Empty =>
                Log.tracef(m =>
                  m("Got empty payload for requestId: %d", requestId)
                );
                Lwt.wakeup(resolver, `Null);
+
              | Bytes(bytes) =>
-               Log.warnf(m =>
-                 m(
-                   "Got %d bytes for requestId: %d, but bytes handler is not implemented",
-                   Bytes.length(bytes),
-                   requestId,
-                 )
-               )
+               Lwt.wakeup(resolver, `String(Bytes.to_string(bytes)))
              }
            });
         Hashtbl.remove(requestIdToReply, requestId);

src/Exthost/Exthost.rei

@@ -863,6 +863,12 @@ module Files: {
     let encode: Json.encoder(t);
   };
 
+  module ReadDirResult: {
+    type t = (string, FileType.t);
+
+    let encode: Json.encoder(t);
+  };
+
   module FileSystemEvents: {
     type t = {
       created: list(Oni_Core.Uri.t),
@@ -1627,10 +1633,7 @@ module Reply: {
   let okBuffer: Bytes.t => t;
 };
 
-module Middleware: {
-  let download: Msg.DownloadService.msg => Lwt.t(Reply.t);
-  let filesystem: Msg.FileSystem.msg => Lwt.t(Reply.t);
-};
+module Middleware: {let download: Msg.DownloadService.msg => Lwt.t(Reply.t);};
 
 module Client: {
   type t;
@@ -1738,6 +1741,10 @@ module Request: {
       Lwt.t(unit);
   };
 
+  module FileSystem: {
+    let readFile: (~handle: int, ~uri: Uri.t, Client.t) => Lwt.t(string);
+  };
+
   module FileSystemEventService: {
     let onFileEvent: (~events: Files.FileSystemEvents.t, Client.t) => unit;
     // TODO

src/Exthost/Middleware.re

@@ -1,5 +1,3 @@
-open Oni_Core;
-
 let download = (msg: Msg.DownloadService.msg) => {
   switch (msg) {
   | Download({uri, dest}) =>
@@ -10,123 +8,3 @@ let download = (msg: Msg.DownloadService.msg) => {
     |> Lwt.map(_ => Reply.okEmpty);
   };
 };
-
-let filesystem = (msg: Msg.FileSystem.msg) => {
-  let mapEncoder = (encoder, v) => {
-    v |> Json.Encode.encode_value(encoder) |> Reply.okJson;
-  };
-  open Msg.FileSystem;
-
-  let fileTypeFromStat: Luv.File.Stat.t => Files.FileType.t =
-    (statResult: Luv.File.Stat.t) => {
-      Luv.File.(
-        Luv.File.Stat.(
-          Files.FileType.(
-            if (Mode.test([`IFREG], statResult.mode)) {
-              File;
-            } else if (Mode.test([`IFDIR], statResult.mode)) {
-              Directory;
-            } else if (Mode.test([`IFLNK], statResult.mode)) {
-              SymbolicLink;
-            } else {
-              Unknown;
-            }
-          )
-        )
-      );
-    };
-
-  let statToExthostStat: Luv.File.Stat.t => Files.StatResult.t =
-    stat => {
-      Files.(
-        (
-          {
-            fileType: fileTypeFromStat(stat),
-            mtime: stat.mtim.sec |> Signed.Long.to_int,
-            ctime: stat.ctim.sec |> Signed.Long.to_int,
-            size: stat.size |> Unsigned.UInt64.to_int,
-          }: StatResult.t
-        )
-      );
-    };
-
-  let dirEntryToExthostDirEntry:
-    Luv.File.Dirent.t => (string, Files.FileType.t) =
-    dirent => {
-      let fileType =
-        switch (dirent.kind) {
-        | `FILE => Files.FileType.File
-        | `DIR => Files.FileType.Directory
-        | `LINK => Files.FileType.SymbolicLink
-        | _ => Files.FileType.Unknown
-        };
-
-      (dirent.name, fileType);
-    };
-
-  switch (msg) {
-  | Stat({uri}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.stat
-    |> Lwt.map(statToExthostStat)
-    |> Lwt.map(mapEncoder(Files.StatResult.encode))
-
-  | ReadDir({uri}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.readdir
-    |> Lwt.map(List.map(dirEntryToExthostDirEntry))
-    |> Lwt.map(mapEncoder(Json.Encode.list(Files.ReadDirResult.encode)))
-
-  | ReadFile({uri}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.readFile
-    |> Lwt.map(Reply.okBuffer)
-
-  | WriteFile({uri, bytes}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.writeFile(~contents=bytes)
-    |> Lwt.map(() => Reply.okEmpty)
-
-  | Rename({source, target, opts}) =>
-    let sourcePath = source |> Uri.toFileSystemPath;
-    let targetPath = target |> Uri.toFileSystemPath;
-
-    Service_OS.Api.rename(
-      ~source=sourcePath,
-      ~target=targetPath,
-      ~overwrite=opts.overwrite,
-    )
-    |> Lwt.map(() => Reply.okEmpty);
-
-  | Copy({source, target, opts}) =>
-    let sourcePath = source |> Uri.toFileSystemPath;
-    let targetPath = target |> Uri.toFileSystemPath;
-
-    Service_OS.Api.rename(
-      ~source=sourcePath,
-      ~target=targetPath,
-      ~overwrite=opts.overwrite,
-    )
-    |> Lwt.map(() => Reply.okEmpty);
-
-  | Mkdir({uri}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.mkdir
-    |> Lwt.map(() => Reply.okEmpty)
-
-  | Delete({uri, opts}) =>
-    uri
-    |> Uri.toFileSystemPath
-    |> Service_OS.Api.rmdir(~recursive=opts.recursive)
-    |> Lwt.map(() => Reply.okEmpty)
-
-  | RegisterFileSystemProvider(_) => Lwt.return(Reply.okEmpty)
-  | UnregisterProvider(_) => Lwt.return(Reply.okEmpty)
-  | OnFileSystemChange(_) => Lwt.return(Reply.okEmpty)
-  };
-};

src/Exthost/Msg.re

@@ -685,6 +685,19 @@ module FileSystem = {
         let%bind opts =
           deleteOptsJson |> Internal.decode_value(FileDeleteOptions.decode);
         Ok(Delete({uri, opts}));
+
+      | (
+          "$registerFileSystemProvider",
+          `List([`Int(handle), `String(scheme), capabilitiesJson]),
+        ) =>
+        let%bind capabilities =
+          capabilitiesJson
+          |> Internal.decode_value(FileSystemProviderCapabilities.decode);
+        Ok(RegisterFileSystemProvider({handle, scheme, capabilities}));
+
+      | ("$unregisterFileSystemProvider", `List([`Int(handle)])) =>
+        Ok(UnregisterProvider({handle: handle}))
+
       | _ => Error("Unhandled FileSystem method: " ++ method)
       }
     );

src/Exthost/Request.re

@@ -210,6 +210,20 @@ module ExtensionService = {
   };
 };
 
+module FileSystem = {
+  open Json.Encode;
+
+  let readFile = (~handle, ~uri, client) => {
+    Client.request(
+      ~decoder=Json.Decode.string,
+      ~rpcName="ExtHostFileSystem",
+      ~method="$readFile",
+      ~args=`List([`Int(handle), uri |> encode_value(Uri.encode)]),
+      client,
+    );
+  };
+};
+
 module FileSystemEventService = {
   open Json.Encode;