From 60e32e5707998150d68817bfa30efb5b6e92b9dc Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Tue, 25 Jun 2019 15:27:39 -0700 Subject: [PATCH] Bugfix: Onivim 2 doesn't free up terminal when running (#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 | 11 +++- src/editor/Core/Keybindings.re | 2 +- src/editor/Core/Log.re | 43 +++++++++++-- src/editor/Core/Utility.re | 2 +- src/editor/Extensions/ExtHostTransport.re | 8 ++- src/editor/Model/Buffers.re | 4 +- .../Store/ExtensionClientStoreConnector.re | 2 +- .../Store/TextmateClientStoreConnector.re | 4 +- src/editor/Store/VimStoreConnector.re | 7 ++- src/editor/{bin => bin_editor}/Input.re | 0 .../Oni2.re => bin_editor/Oni2_editor.re} | 11 ++++ .../{bin => bin_editor}/PreflightChecks.re | 0 src/editor/{bin => bin_editor}/dune | 4 +- src/editor/bin_launcher/Oni2.re | 60 +++++++++++++++++++ src/editor/bin_launcher/dune | 8 +++ 15 files changed, 144 insertions(+), 22 deletions(-) rename src/editor/{bin => bin_editor}/Input.re (100%) rename src/editor/{bin/Oni2.re => bin_editor/Oni2_editor.re} (91%) rename src/editor/{bin => bin_editor}/PreflightChecks.re (100%) rename src/editor/{bin => bin_editor}/dune (87%) create mode 100644 src/editor/bin_launcher/Oni2.re create mode 100644 src/editor/bin_launcher/dune diff --git a/src/editor/Core/Cli.re b/src/editor/Core/Cli.re index 6d24a5294a..6790f9784a 100644 --- a/src/editor/Core/Cli.re +++ b/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(); diff --git a/src/editor/Core/Keybindings.re b/src/editor/Core/Keybindings.re index d305a1cdac..894f3646be 100644 --- a/src/editor/Core/Keybindings.re +++ b/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); []; }; }; diff --git a/src/editor/Core/Log.re b/src/editor/Core/Log.re index ad1bd4aca3..5348c3fffb 100644 --- a/src/editor/Core/Log.re +++ b/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); diff --git a/src/editor/Core/Utility.re b/src/editor/Core/Utility.re index c937be1eb6..a58eb07270 100644 --- a/src/editor/Core/Utility.re +++ b/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; }; diff --git a/src/editor/Extensions/ExtHostTransport.re b/src/editor/Extensions/ExtHostTransport.re index 7dd535397b..4fc9ced09e 100644 --- a/src/editor/Extensions/ExtHostTransport.re +++ b/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) }; }; diff --git a/src/editor/Model/Buffers.re b/src/editor/Model/Buffers.re index 294fb0e8dc..cfde864af9 100644 --- a/src/editor/Model/Buffers.re +++ b/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, ); diff --git a/src/editor/Store/ExtensionClientStoreConnector.re b/src/editor/Store/ExtensionClientStoreConnector.re index 15e06238b2..2cb372d33c 100644 --- a/src/editor/Store/ExtensionClientStoreConnector.re +++ b/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 diff --git a/src/editor/Store/TextmateClientStoreConnector.re b/src/editor/Store/TextmateClientStoreConnector.re index 841eb8d425..cff3911b8e 100644 --- a/src/editor/Store/TextmateClientStoreConnector.re +++ b/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)); diff --git a/src/editor/Store/VimStoreConnector.re b/src/editor/Store/VimStoreConnector.re index e60f2db955..9e4be2fb88 100644 --- a/src/editor/Store/VimStoreConnector.re +++ b/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, "") && !String.equal(key, "") && !String.equal(key, "")) { + 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); }); diff --git a/src/editor/bin/Input.re b/src/editor/bin_editor/Input.re similarity index 100% rename from src/editor/bin/Input.re rename to src/editor/bin_editor/Input.re diff --git a/src/editor/bin/Oni2.re b/src/editor/bin_editor/Oni2_editor.re similarity index 91% rename from src/editor/bin/Oni2.re rename to src/editor/bin_editor/Oni2_editor.re index 041b900ed5..2fcdb0bd8f 100644 --- a/src/editor/bin/Oni2.re +++ b/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,6 +65,7 @@ let init = app => { update(); }; + Log.debug("Startup: Starting StoreThread"); let (dispatch, runEffects) = Store.StoreThread.start( ~setup, @@ -64,6 +73,7 @@ let init = app => { ~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); diff --git a/src/editor/bin/PreflightChecks.re b/src/editor/bin_editor/PreflightChecks.re similarity index 100% rename from src/editor/bin/PreflightChecks.re rename to src/editor/bin_editor/PreflightChecks.re diff --git a/src/editor/bin/dune b/src/editor/bin_editor/dune similarity index 87% rename from src/editor/bin/dune rename to src/editor/bin_editor/dune index 506c66e309..412af05b24 100644 --- a/src/editor/bin/dune +++ b/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 diff --git a/src/editor/bin_launcher/Oni2.re b/src/editor/bin_launcher/Oni2.re new file mode 100644 index 0000000000..63f8871f1f --- /dev/null +++ b/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(); diff --git a/src/editor/bin_launcher/dune b/src/editor/bin_launcher/dune new file mode 100644 index 0000000000..2f5a4a6497 --- /dev/null +++ b/src/editor/bin_launcher/dune @@ -0,0 +1,8 @@ +(ignored_subdirs (node_modules _esy)) + +(executable + (name Oni2) + (package Oni2) + (public_name Oni2) + (libraries unix) +)