Merge pull request #302 from garden-io/empty-log-entries

fix(logger): let empty entries inherit parent indentation level

garden-cli/src/logger/log-entry.ts

@@ -29,6 +29,7 @@ export interface UpdateOpts {
   showDuration?: boolean
   error?: GardenError
   status?: EntryStatus
+  indentationLevel?: number
 }
 
 export interface CreateOpts extends UpdateOpts {
@@ -40,20 +41,9 @@ export type CreateParam = string | CreateOpts
 export interface LogEntryConstructor {
   level: LogLevel
   opts: CreateOpts
-  depth: number
   parent: LogNode
 }
 
-export function createLogEntry(level: LogLevel, parent: LogNode, opts: CreateOpts): LogEntry {
-  const params: LogEntryConstructor = {
-    level,
-    opts,
-    parent,
-    depth: parent.depth + 1,
-  }
-  return new LogEntry(params)
-}
-
 // TODO Fix any cast
 export function resolveParam<T extends UpdateOpts>(param?: string | T): T {
   return typeof param === "string" ? <any>{ msg: param } : param || {}
@@ -62,9 +52,9 @@ export function resolveParam<T extends UpdateOpts>(param?: string | T): T {
 export class LogEntry extends LogNode {
   public opts: UpdateOpts
 
-  constructor({ level, opts, depth, parent }: LogEntryConstructor) {
+  constructor({ level, opts, parent }: LogEntryConstructor) {
     const { id, ...otherOpts } = opts
-    super(level, depth, parent, id)
+    super(level, parent, id)
     this.opts = otherOpts
     if (this.level === LogLevel.error) {
       this.opts.status = "error"
@@ -110,11 +100,21 @@ export class LogEntry extends LogNode {
   }
 
   createNode(level: LogLevel, parent: LogNode, param?: CreateParam) {
-    return createLogEntry(level, parent, resolveParam(param))
+    // Empty entries inherit their parent's indentation level
+    let { indentationLevel } = this.opts
+    if (param) {
+      indentationLevel = (indentationLevel || 0) + 1
+    }
+    const opts = {
+      indentationLevel,
+      ...resolveParam(param),
+    }
+    return new LogEntry({ level, opts, parent })
   }
 
+  // Preserves status
   setState(param?: string | UpdateOpts): LogEntry {
-    this.deepSetState(resolveParam(param))
+    this.deepSetState({ ...resolveParam(param), status: this.opts.status })
     this.root.onGraphChange(this)
     return this
   }

garden-cli/src/logger/log-node.ts

@@ -29,7 +29,6 @@ export abstract class LogNode<T = LogEntry, U = CreateParam> {
 
   constructor(
     public readonly level: LogLevel,
-    public readonly depth: number,
     public readonly parent?: LogNode<T>,
     public readonly id?: string,
   ) {
@@ -46,35 +45,35 @@ export abstract class LogNode<T = LogEntry, U = CreateParam> {
 
   abstract createNode(level: LogLevel, parent: LogNode<T, U>, param?: U): T
 
-  protected addNode(level: LogLevel, param?: U): T {
+  protected appendNode(level: LogLevel, param?: U): T {
     const node = this.createNode(level, this, param)
     this.children.push(node)
     this.root.onGraphChange(node)
     return node
   }
 
   silly(param?: U): T {
-    return this.addNode(LogLevel.silly, param)
+    return this.appendNode(LogLevel.silly, param)
   }
 
   debug(param?: U): T {
-    return this.addNode(LogLevel.debug, param)
+    return this.appendNode(LogLevel.debug, param)
   }
 
   verbose(param?: U): T {
-    return this.addNode(LogLevel.verbose, param)
+    return this.appendNode(LogLevel.verbose, param)
   }
 
   info(param?: U): T {
-    return this.addNode(LogLevel.info, param)
+    return this.appendNode(LogLevel.info, param)
   }
 
   warn(param?: U): T {
-    return this.addNode(LogLevel.warn, param)
+    return this.appendNode(LogLevel.warn, param)
   }
 
   error(param?: U): T {
-    return this.addNode(LogLevel.error, param)
+    return this.appendNode(LogLevel.error, param)
   }
 
   /**

garden-cli/src/logger/logger.ts

@@ -10,7 +10,7 @@ import * as nodeEmoji from "node-emoji"
 import chalk from "chalk"
 
 import { RootLogNode, LogNode } from "./log-node"
-import { LogEntry, CreateOpts, createLogEntry, resolveParam } from "./log-entry"
+import { LogEntry, CreateOpts, resolveParam } from "./log-entry"
 import { getChildEntries } from "./util"
 import { Writer } from "./writers/base"
 import { InternalError, ParameterError } from "../exceptions"
@@ -19,8 +19,6 @@ import { FancyTerminalWriter } from "./writers/fancy-terminal-writer"
 import { BasicTerminalWriter } from "./writers/basic-terminal-writer"
 import { combine } from "./renderers"
 
-const ROOT_DEPTH = -1
-
 export enum LoggerType {
   quiet = "quiet",
   basic = "basic",
@@ -90,12 +88,12 @@ export class Logger extends RootLogNode<LogEntry> {
   }
 
   private constructor(config: LoggerConfig) {
-    super(config.level, ROOT_DEPTH)
+    super(config.level)
     this.writers = config.writers || []
   }
 
   createNode(level: LogLevel, _parent: LogNode, opts: CreateOpts) {
-    return createLogEntry(level, this, resolveParam(opts))
+    return new LogEntry({ level, parent: this, opts: resolveParam(opts) })
   }
 
   onGraphChange(entry: LogEntry) {

garden-cli/src/logger/renderers.ts

@@ -65,7 +65,7 @@ export function combine(renderers: Renderers): string {
 
 /*** RENDERERS ***/
 export function leftPad(entry: LogEntry): string {
-  return padStart("", entry.depth * 3)
+  return padStart("", (entry.opts.indentationLevel || 0) * 3)
 }
 
 export function renderEmoji(entry: LogEntry): string {
@@ -129,26 +129,13 @@ export function renderDuration(entry: LogEntry): string {
 }
 
 export function formatForTerminal(entry: LogEntry): string {
-  let renderers
-  if (entry.depth > 0) {
-    // Skip section on child entries.
-    renderers = [
-      [leftPad, [entry]],
-      [renderSymbol, [entry]],
-      [renderEmoji, [entry]],
-      [renderMsg, [entry]],
-      [renderDuration, [entry]],
-      ["\n"],
-    ]
-  } else {
-    renderers = [
-      [renderSymbol, [entry]],
-      [renderSection, [entry]],
-      [renderEmoji, [entry]],
-      [renderMsg, [entry]],
-      [renderDuration, [entry]],
-      ["\n"],
-    ]
-  }
-  return combine(renderers)
+  return combine([
+    [leftPad, [entry]],
+    [renderSymbol, [entry]],
+    [renderSection, [entry]],
+    [renderEmoji, [entry]],
+    [renderMsg, [entry]],
+    [renderDuration, [entry]],
+    ["\n"],
+  ])
 }

garden-cli/src/logger/writers/fancy-terminal-writer.ts

@@ -20,11 +20,16 @@ import {
 import { LogEntry } from "../log-entry"
 import { Logger } from "../logger"
 import { LogLevel } from "../log-node"
-import { getChildEntries, getTerminalWidth, interceptStream, validate } from "../util"
+import {
+  getChildEntries,
+  getTerminalWidth,
+  interceptStream,
+  validate,
+} from "../util"
 import { Writer, WriterConfig } from "./base"
 
-const FANCY_LOGGER_UPDATE_FREQUENCY_MS = 60
-const FANCY_LOGGER_THROTTLE_MS = 600
+const INTERVAL_MS = 60
+const THROTTLE_MS = 600
 
 const spinnerStyle = chalk.cyan
 
@@ -105,7 +110,7 @@ export class FancyTerminalWriter extends Writer {
     this.stopLoop()
     this.intervalID = setInterval(
       () => this.spin(entries, totalLines),
-      FANCY_LOGGER_UPDATE_FREQUENCY_MS,
+      INTERVAL_MS,
     )
   }
 
@@ -139,7 +144,7 @@ export class FancyTerminalWriter extends Writer {
     // Suspend processing and write immediately if a lot of data is being intercepted, e.g. when user is typing in input
     if (logEntry.fromStdStream() && !didWrite) {
       const now = Date.now()
-      const throttleProcessing = this.lastInterceptAt && (now - this.lastInterceptAt) < FANCY_LOGGER_THROTTLE_MS
+      const throttleProcessing = this.lastInterceptAt && (now - this.lastInterceptAt) < THROTTLE_MS
       this.lastInterceptAt = now
 
       if (throttleProcessing) {
@@ -152,7 +157,7 @@ export class FancyTerminalWriter extends Writer {
           if (this.updatePending) {
             this.handleGraphChange(logEntry, logger, true)
           }
-        }, FANCY_LOGGER_THROTTLE_MS)
+        }, THROTTLE_MS)
         return
       }
     }

garden-cli/test/src/logger.ts

@@ -36,6 +36,27 @@ describe("LogNode", () => {
     })
   })
 
+  describe("appendNode", () => {
+    it("should add new child entries to the respective node", () => {
+      logger.error("error")
+      logger.warn("warn")
+      logger.info("info")
+      logger.verbose("verbose")
+      logger.debug("debug")
+      logger.silly("silly")
+
+      const prevLength = logger.children.length
+      const entry = logger.children[0]
+      const nested = entry.info("nested")
+      const deepNested = nested.info("deep")
+
+      expect(logger.children[0].children).to.have.lengthOf(1)
+      expect(logger.children[0].children[0]).to.eql(nested)
+      expect(logger.children[0].children[0].children[0]).to.eql(deepNested)
+      expect(logger.children).to.have.lengthOf(prevLength)
+    })
+  })
+
 })
 
 describe("RootLogNode", () => {
@@ -63,28 +84,6 @@ describe("RootLogNode", () => {
     })
   })
 
-  describe("addNode", () => {
-    it("should add new child entries to the respective node", () => {
-      logger.error("error")
-      logger.warn("warn")
-      logger.info("info")
-      logger.verbose("verbose")
-      logger.debug("debug")
-      logger.silly("silly")
-
-      const prevLength = logger.children.length
-      const entry = logger.children[0]
-      const nested = entry.info("nested")
-      const deepNested = nested.info("deep")
-
-      expect(logger.children[0].children).to.have.lengthOf(1)
-      expect(logger.children[0].children[0]).to.eql(nested)
-      expect(logger.children[0].children[0].children[0]).to.eql(deepNested)
-      expect(logger.children).to.have.lengthOf(prevLength)
-      expect(deepNested["depth"]).to.equal(2)
-    })
-  })
-
 })
 
 describe("Writers", () => {
@@ -159,13 +158,35 @@ describe("Writers", () => {
 })
 
 describe("LogEntry", () => {
-  const entry = logger.info("")
+  const entry = logger.info()
+  describe("createNode", () => {
+    it("should set the correct indentation level", () => {
+      const nested = entry.info("nested")
+      const deepNested = nested.info("deep nested")
+      const deepDeepNested = deepNested.info("deep deep inside")
+      const deepDeepEmpty = deepDeepNested.info()
+      const indentations = [
+        nested.opts.indentationLevel,
+        deepNested.opts.indentationLevel,
+        deepDeepNested.opts.indentationLevel,
+        deepDeepEmpty.opts.indentationLevel,
+      ]
+      expect(indentations).to.eql([1, 2, 3, 3])
+    })
+  })
   describe("setState", () => {
     it("should update entry state and optionally append new msg to previous msg", () => {
       entry.setState("new")
-      expect(entry["opts"]["msg"]).to.equal("new")
+      expect(entry.opts.msg).to.equal("new")
       entry.setState({ msg: "new2", append: true })
-      expect(entry["opts"]["msg"]).to.eql(["new", "new2"])
+      expect(entry.opts.msg).to.eql(["new", "new2"])
+    })
+  })
+  describe("setState", () => {
+    it("should preserve status", () => {
+      entry.setSuccess()
+      entry.setState("change text")
+      expect(entry.opts.status).to.equal("success")
     })
   })
   describe("setDone", () => {