Dependency: revery -> 1ccda2b; timber -> ae065bb (onivim#1247)

* Dependency: revery -> 6a9fbd6; timber -> ae065bb * tweak log levels, use trace insteaf of debug where appropriate * add --trace command line option * update contributing doc with new log level * Dependency: revery -> 1ccda2b
aubinlrx · Jan 24, 2020 · d6f5116 · d6f5116
1 parent 8b808e9
commit d6f5116
Show file tree

Hide file tree

Showing 24 changed files with 112 additions and 103 deletions.
diff --git a/bench.esy.lock/index.json b/bench.esy.lock/index.json
diff --git a/docs/docs/for-developers/contributing.md b/docs/docs/for-developers/contributing.md
@@ -47,6 +47,7 @@ The following conventions are used to determine the priority level of log messag
 - __Warn__ if it MIGHT cause unexpected behaviour, but also might not, i.e. it's suspicious, but not a definite problem.
 - __Info__ if it is both understandable AND actionable by the end-user
 - __Debug__ otherwise
+- __Trace__ if the output is overwhelming or just excessively detailed. Namespace filtering is expected at this level
 
 ### Branch Naming
 

diff --git a/esy.lock/index.json b/esy.lock/index.json
diff --git a/integration_test/lib/Oni_IntegrationTestLib.re b/integration_test/lib/Oni_IntegrationTestLib.re
@@ -5,6 +5,7 @@ module Option = Utility.Option;
 module Model = Oni_Model;
 module Store = Oni_Store;
 module Log = (val Core.Log.withNamespace("IntegrationTest"));
+module InitLog = (val Core.Log.withNamespace("IntegrationTest.Init"));
 module TextSynchronization = TextSynchronization;
 module ExtensionHelpers = ExtensionHelpers;
 
@@ -63,8 +64,8 @@ let runTest =
   };
 
   Printexc.record_backtrace(true);
-  Timber.App.enablePrinting();
-  Timber.App.enableDebugLogging();
+  Timber.App.enable();
+  Timber.App.setLevel(Timber.Level.trace);
 
   Log.info("Starting test... Working directory: " ++ Sys.getcwd());
 
@@ -77,14 +78,12 @@ let runTest =
     currentState := v;
   };
 
-  let logInit = s => Log.debug("[INITIALIZATION] " ++ s);
-
-  logInit("Starting store...");
+  InitLog.info("Starting store...");
 
   let configurationFilePath = Filename.temp_file("configuration", ".json");
   let oc = open_out(configurationFilePath);
 
-  logInit("Writing configuration file: " ++ configurationFilePath);
+  InitLog.info("Writing configuration file: " ++ configurationFilePath);
 
   let () =
     configuration
@@ -114,9 +113,9 @@ let runTest =
       (),
     );
 
-  logInit("Store started!");
+  InitLog.info("Store started!");
 
-  logInit("Sending init event");
+  InitLog.info("Sending init event");
 
   dispatch(Model.Actions.Init);
 

diff --git a/integrationtest.esy.lock/index.json b/integrationtest.esy.lock/index.json
diff --git a/package.json b/package.json
@@ -240,13 +240,13 @@
     "@opam/merlin-extend": "0.4",
     "rench": "github:bryphe/rench#4860ef4",
     "reason-tree-sitter": "github:onivim/reason-tree-sitter#e2c571e",
-    "revery": "github:revery-ui/revery#eeda678",
+    "revery": "revery-ui/revery#1ccda2b",
     "reason-libvim": "onivim/reason-libvim#3782892",
     "editor-core-types": "onivim/editor-core-types#16dd98e",
     "esy-cmake": "prometheansacrifice/esy-cmake#2a47392def755",
     "esy-sdl2": "revery-ui/esy-sdl2#b63892b",
     "esy-skia": "revery-ui/esy-skia#91b10c9",
-    "timber": "glennsl/timber#ae5de6c",
+    "timber": "glennsl/timber#ae065bb",
     "yarn-pkg-config": "esy-ocaml/yarn-pkg-config#db3a0b6"
   },
   "devDependencies": {

diff --git a/src/Core/Cli.re b/src/Core/Cli.re
@@ -96,9 +96,10 @@ let parse =
 
   Arg.parse(
     [
-      ("-f", Unit(Timber.App.enablePrinting), ""),
-      ("--nofork", Unit(Timber.App.enablePrinting), ""),
-      ("--debug", Unit(CoreLog.enableDebugLogging), ""),
+      ("-f", Unit(Timber.App.enable), ""),
+      ("--nofork", Unit(Timber.App.enable), ""),
+      ("--debug", Unit(CoreLog.enableDebug), ""),
+      ("--trace", Unit(CoreLog.enableTrace), ""),
       ("--version", printVersion |> runAndExitUnit, ""),
       ("--no-log-colors", Unit(Timber.App.disableColors), ""),
       ("--disable-extensions", Unit(disableExtensionLoading), ""),
@@ -128,7 +129,7 @@ let parse =
     "",
   );
 
-  if (CoreLog.isPrintingEnabled() || needsConsole^) {
+  if (Timber.App.isEnabled() || needsConsole^) {
     /* On Windows, we need to create a console instance if possible */
     Revery.App.initConsole();
   };

diff --git a/src/Core/Job.re b/src/Core/Job.re
@@ -91,15 +91,15 @@ let tick = (~budget=None, v: t('p, 'c)) => {
   let current = ref(v);
   let iterations = ref(0);
 
-  Log.debug("Starting " ++ v.name);
+  Log.trace("Starting " ++ v.name);
   while (Unix.gettimeofday() -. startTime < budget && !current^.isComplete) {
     current := doWork(current^);
     incr(iterations);
   };
 
   let endTime = Unix.gettimeofday();
 
-  Log.debugf(m =>
+  Log.tracef(m =>
     m(
       "%s ran %i iterations for %fs",
       v.name,
@@ -108,7 +108,7 @@ let tick = (~budget=None, v: t('p, 'c)) => {
     )
   );
 
-  Log.debugf(m => m("Detailed report: %s", show(v)));
+  Log.tracef(m => m("Detailed report: %s", show(v)));
 
   current^;
 };
diff --git a/src/Core/Kernel/Log.re b/src/Core/Kernel/Log.re
@@ -1,5 +1,3 @@
-include Timber;
-
 // TODO: Remove after 4.08
 module Option = {
   let iter = f =>
@@ -8,6 +6,10 @@ module Option = {
     | None => ();
 };
 
+include Timber.Log;
+
+module type Logger = Timber.Logger;
+
 module Env = {
   let logFile = Sys.getenv_opt("ONI2_LOG_FILE");
   let debug = Sys.getenv_opt("ONI2_DEBUG");
@@ -33,8 +35,8 @@ let writeExceptionLog = (e, bt) => {
   Stdlib.close_out(oc);
 };
 
-let enableDebugLogging = () => {
-  Timber.App.enableDebugLogging();
+let enableDebug = () => {
+  Timber.App.setLevel(Timber.Level.debug);
   module Log = (val withNamespace("Oni2.Exception"));
 
   Log.debug("Recording backtraces");
@@ -52,8 +54,13 @@ let enableDebugLogging = () => {
   });
 };
 
-if (Timber.isDebugLoggingEnabled()) {
-  enableDebugLogging();
+let enableTrace = () => {
+  enableDebug();
+  Timber.App.setLevel(Timber.Level.trace);
+};
+
+if (Timber.App.isLevelEnabled(Timber.Level.debug)) {
+  enableDebug();
 } else {
   // Even if we're not debugging.... at least emit the exception
   Printexc.set_uncaught_exception_handler((e, bt) => {

diff --git a/src/Core/Kernel/Log.rei b/src/Core/Kernel/Log.rei
@@ -1,8 +1,4 @@
-let isPrintingEnabled: unit => bool;
-let isDebugLoggingEnabled: unit => bool;
-
 let perf: (string, unit => 'a) => 'a;
-let fn: (string, 'a => 'b, 'a) => 'b;
 
 module type Logger = {
   let errorf: Timber.msgf(_, unit) => unit;
@@ -13,8 +9,12 @@ module type Logger = {
   let info: string => unit;
   let debugf: Timber.msgf(_, unit) => unit;
   let debug: string => unit;
-  let fn: (string, 'a => 'b, 'a) => 'b;
+  let tracef: Timber.msgf(_, unit) => unit;
+  let trace: string => unit;
+  let fn: (string, 'a => 'b, ~pp: 'b => string=?, 'a) => 'b;
 };
 
-let enableDebugLogging: unit => unit;
 let withNamespace: string => (module Logger);
+
+let enableDebug: unit => unit;
+let enableTrace: unit => unit;
diff --git a/src/Extensions/ExtHostTransport.re b/src/Extensions/ExtHostTransport.re
@@ -136,7 +136,7 @@ let start =
     };
 
   let handleReply = (reqId: int, payload: Yojson.Safe.t) => {
-    Log.debugf(m =>
+    Log.tracef(m =>
       m("Reply ID: %d payload: %s\n", reqId, Yojson.Safe.to_string(payload))
     );
     switch (Hashtbl.find_opt(replyIdToResolver, reqId)) {

diff --git a/src/Feature/Search/SearchSubscription.re b/src/Feature/Search/SearchSubscription.re
@@ -24,7 +24,7 @@ module Make = (Config: {type action;}) => {
           ~params as {directory, query, ripgrep, onUpdate, onCompleted},
           ~dispatch: _,
         ) => {
-      Log.debug("Starting " ++ id);
+      Log.info("Starting " ++ id);
 
       let dispose =
         ripgrep.Ripgrep.findInFiles(

diff --git a/src/Input/Expression.re b/src/Input/Expression.re
@@ -42,7 +42,7 @@ let evaluate = (v: t, getValue) => {
 
   let ret = eval(v);
 
-  Log.debugf(m =>
+  Log.tracef(m =>
     m("Expression %s evaluated to: %s", toString(v), ret ? "true" : "false")
   );