Bugfix: Onivim 2 doesn't free up terminal when running (onivim#335)

* Create separate process launcher process to free terminal

* Create logging infra for when running in detached mode

* Formatting

* Fix problematic logging

* Formatting

* Remove debug printing

* Switch from print to using the log utility

* Formatting

* Fix error

* Formatting

* Fix for non-windows platforms

* Formatting

* Move close_on_exec after

* Only allow printing if running in foreground

* Formatting

src/editor/Core/Cli.re

@@ -23,10 +23,19 @@ let show = (v: t) => {
   "Folder: " ++ v.folder ++ newline ++ "Files: " ++ files;
 };
 
+let noop = () => ();
+
 let parse = (setup: Setup.t) => {
   let args: ref(list(string)) = ref([]);
 
-  Arg.parse([], arg => args := [arg, ...args^], header(setup.version));
+  Arg.parse(
+    [
+      ("-f", Unit(Log.enablePrinting), ""),
+      ("--nofork", Unit(Log.enablePrinting), ""),
+    ],
+    arg => args := [arg, ...args^],
+    header(setup.version),
+  );
 
   let paths = args^ |> List.rev;
   let workingDirectory = Environment.getWorkingDirectory();

src/editor/Core/Keybindings.re

@@ -28,7 +28,7 @@ let get = () => {
   switch (ofFile(getBundledKeybindingsPath())) {
   | Ok(b) => b.bindings
   | Error(e) =>
-    print_endline("Error parsing keybindings file ------- " ++ e);
+    Log.error("Error parsing keybindings file ------- " ++ e);
     [];
   };
 };

src/editor/Core/Log.re

@@ -4,15 +4,46 @@
  * Simple console logger
  */
 
+let canPrint = ref(false);
+
+let enablePrinting = () => canPrint := true;
+
+let print = msg => canPrint^ ? print_endline(msg) : ();
+
+let prerr = msg => canPrint^ ? prerr_endline(msg) : ();
+
+let fileChannel =
+  switch (Sys.getenv_opt("ONI2_LOG_FILE")) {
+  | Some(v) =>
+    let oc = open_out(v);
+    Printf.fprintf(oc, "Starting log file.\n");
+    flush(oc);
+    Some(oc);
+  | None => None
+  };
+
 let isDebugLoggingEnabled =
-  switch (Sys.getenv_opt("ONI2_DEBUG")) {
-  | Some(_) => true
+  switch (Sys.getenv_opt("ONI2_DEBUG"), Sys.getenv_opt("ONI2_LOG_FILE")) {
+  | (Some(_), _) => true
+  | (_, Some(_)) => true
   | _ => false
   };
 
-let info = msg => print_endline("[INFO] " ++ msg);
+let logCore = (~error=false, msg) => {
+  switch (fileChannel) {
+  | Some(oc) =>
+    Printf.fprintf(oc, "%s\n", msg);
+    flush(oc);
+  | None =>
+    switch (error) {
+    | false => print(msg)
+    | true => prerr(msg)
+    }
+  };
+};
+
+let info = msg => logCore("[INFO] " ++ msg);
 
-let debug = msg =>
-  isDebugLoggingEnabled ? print_endline("[DEBUG] " ++ msg) : ();
+let debug = msg => isDebugLoggingEnabled ? logCore("[DEBUG] " ++ msg) : ();
 
-let error = msg => prerr_endline("[ERROR] " ++ msg);
+let error = msg => logCore(~error=true, "[ERROR] " ++ msg);

src/editor/Core/Utility.re

@@ -40,7 +40,7 @@ let convertUTF8string = str =>
 let safe_fold_left2 = (fn, accum, list1, list2, ~default) =>
   try (List.fold_left2(fn, accum, list1, list2)) {
   | Invalid_argument(reason) =>
-    print_endline("fold_left2 failing because: " ++ reason);
+    Log.error("fold_left2 failing because: " ++ reason);
     default;
   };
 

src/editor/Extensions/ExtHostTransport.re

@@ -102,7 +102,10 @@ let start =
       | _ => sendResponse(12, reqId, `Assoc([]))
       }
     | _ =>
-      print_endline("Unknown message: " ++ Yojson.Safe.to_string(payload))
+      Log.error(
+        "ExtHostTransport - Unknown message: "
+        ++ Yojson.Safe.to_string(payload),
+      )
     };
 
   let _sendInitData = () => {
@@ -137,8 +140,7 @@ let start =
         }
       )
 
-    | _ =>
-      print_endline("[Extension Host Client] Unknown message: " ++ n.method)
+    | _ => Log.error("[Extension Host Client] Unknown message: " ++ n.method)
     };
   };
 

src/editor/Model/Buffers.re

@@ -19,9 +19,7 @@ let remove = IntMap.remove;
 let log = (m: t) =>
   IntMap.iter(
     (_, b) =>
-      Buffer.show(b)
-      |> (++)("Buffer ======================: \n")
-      |> print_endline,
+      Buffer.show(b) |> (++)("Buffer ======================: \n") |> Log.info,
     m,
   );
 

src/editor/Store/ExtensionClientStoreConnector.re

@@ -16,7 +16,7 @@ module Protocol = Extensions.ExtHostProtocol;
 let start = (extensions, setup: Core.Setup.t) => {
   let (stream, dispatch) = Isolinear.Stream.create();
 
-  let onExtHostClosed = () => print_endline("ext host closed");
+  let onExtHostClosed = () => Core.Log.info("ext host closed");
 
   let extensionInfo =
     extensions

src/editor/Store/TextmateClientStoreConnector.re

@@ -13,13 +13,15 @@ module Model = Oni_Model;
 
 module Extensions = Oni_Extensions;
 
+module Log = Core.Log;
+
 let start = (languageInfo: Model.LanguageInfo.t, setup: Core.Setup.t) => {
   let defaultThemePath =
     setup.bundledExtensionsPath ++ "/onedark-pro/themes/OneDark-Pro.json";
 
   let (stream, dispatch) = Isolinear.Stream.create();
 
-  let onScopeLoaded = s => prerr_endline("Scope loaded: " ++ s);
+  let onScopeLoaded = s => Log.debug("Scope loaded: " ++ s);
   let onColorMap = cm => dispatch(Model.Actions.SyntaxHighlightColorMap(cm));
   let onTokens = tr => dispatch(Model.Actions.SyntaxHighlightTokens(tr));
 

src/editor/Store/VimStoreConnector.re

@@ -10,6 +10,8 @@ module Core = Oni_Core;
 module Extensions = Oni_Extensions;
 module Model = Oni_Model;
 
+module Log = Core.Log;
+
 let start = () => {
   let (stream, dispatch) = Isolinear.Stream.create();
 
@@ -173,7 +175,9 @@ let start = () => {
         if (!String.equal(key, "<S-SHIFT>")
             && !String.equal(key, "<C->")
             && !String.equal(key, "<A-C->")) {
+          Log.debug("VimStoreConnector - handling key: " ++ key);
           Vim.input(key);
+          Log.debug("VimStoreConnector - handled key: " ++ key);
         }
       );
 
@@ -190,9 +194,6 @@ let start = () => {
         | Spaces => true
         };
 
-      print_endline(
-        "Setting insert spaces: " ++ string_of_bool(insertSpaces),
-      );
       Vim.Options.setTabSize(indentation.size);
       Vim.Options.setInsertSpaces(insertSpaces);
     });

src/editor/bin/Oni2.re → src/editor/bin_editor/Oni2_editor.re

@@ -14,6 +14,7 @@ open Oni_UI;
 module Core = Oni_Core;
 module Model = Oni_Model;
 module Store = Oni_Store;
+module Log = Core.Log;
 
 /**
    This allows a stack trace to be printed when exceptions occur
@@ -24,6 +25,8 @@ module Store = Oni_Store;
 /* }; */
 Printexc.record_backtrace(true);
 
+let () = Log.debug("Starting Onivim 2.");
+
 /* The 'main' function for our app */
 let init = app => {
   let w =
@@ -38,8 +41,13 @@ let init = app => {
       "Oni2",
     );
 
+  let () = Log.debug("Initializing setup.");
   let setup = Core.Setup.init();
+  Log.debug("Startup: Parsing CLI options");
   let cliOptions = Core.Cli.parse(setup);
+  Log.debug("Startup: Parsing CLI options complete");
+
+  Log.debug("Startup: Changing folder to: " ++ cliOptions.folder);
   Sys.chdir(cliOptions.folder);
 
   PreflightChecks.run();
@@ -57,13 +65,15 @@ let init = app => {
     update(<Root state />);
   };
 
+  Log.debug("Startup: Starting StoreThread");
   let (dispatch, runEffects) =
     Store.StoreThread.start(
       ~setup,
       ~executingDirectory=Revery.Environment.getExecutingDirectory(),
       ~onStateChanged,
       (),
     );
+  Log.debug("Startup: StoreThread started!");
 
   GlobalContext.set({
     getState: () => currentState^,
@@ -171,4 +181,5 @@ let init = app => {
 };
 
 /* Let's get this party started! */
+let () = Log.debug("Calling App.start");
 App.start(init);

src/editor/bin/dune → src/editor/bin_editor/dune

@@ -1,9 +1,9 @@
 (ignored_subdirs (node_modules _esy))
 
 (executable
-    (name Oni2)
+    (name Oni2_editor)
     (package Oni2)
-    (public_name Oni2)
+    (public_name Oni2_editor)
     (libraries
         bigarray
         zed

src/editor/bin_launcher/Oni2.re

@@ -0,0 +1,60 @@
+/*
+ * Oni2.re
+ *
+ * This is the launcher for the editor.
+ */
+
+let stayAttached = ref(false);
+
+let version = () => {
+  print_endline("Onivim 2 0.0.0");
+};
+
+let spec = [
+  ("-f", Arg.Set(stayAttached), ""),
+  ("--nofork", Arg.Set(stayAttached), ""),
+  ("-version", Arg.Unit(version), ""),
+];
+
+let anonArg = _ => ();
+
+let () = Arg.parse(spec, anonArg, "Usage: ");
+
+let executable = Sys.win32 ? "Oni2_editor.exe" : "Oni2_editor";
+
+let startProcess = (stdio, stdout, stderr) => {
+  let executingDirectory = Filename.dirname(Sys.argv[0]);
+  Unix.create_process(
+    executingDirectory ++ "/" ++ executable,
+    Sys.argv,
+    stdio,
+    stdout,
+    stderr,
+  );
+};
+
+let launch = () =>
+  if (stayAttached^) {
+    let pid = startProcess(Unix.stdin, Unix.stdout, Unix.stderr);
+    let _ = Unix.waitpid([], pid);
+    ();
+  } else {
+    let (pstdin, stdin) = Unix.pipe();
+    let (stdout, pstdout) = Unix.pipe();
+    let (stderr, pstderr) = Unix.pipe();
+
+    let _ = startProcess(pstdin, pstdout, pstderr);
+
+    Unix.set_close_on_exec(pstdin);
+    Unix.set_close_on_exec(stdin);
+    Unix.set_close_on_exec(pstdout);
+    Unix.set_close_on_exec(stdout);
+    Unix.set_close_on_exec(pstderr);
+    Unix.set_close_on_exec(stderr);
+
+    Unix.close(pstdin);
+    Unix.close(pstdout);
+    Unix.close(pstderr);
+  };
+
+launch();

src/editor/bin_launcher/dune

@@ -0,0 +1,8 @@
+(ignored_subdirs (node_modules _esy))
+
+(executable
+    (name Oni2)
+    (package Oni2)
+    (public_name Oni2)
+    (libraries unix)
+)