fix(logger): render non-empty entries even though msg is missing

garden-service/src/logger/renderers.ts

@@ -126,8 +126,13 @@ export function renderMsg(entry: LogEntry): string {
 }
 
 export function renderSection(entry: LogEntry): string {
-  const { section } = entry.opts
-  return section ? `${sectionStyle(section)} → ` : ""
+  const { msg, section } = entry.opts
+  if (section && msg) {
+    return `${sectionStyle(section)} → `
+  } else if (section) {
+    return sectionStyle(section)
+  }
+  return ""
 }
 
 export function renderDuration(entry: LogEntry): string {
@@ -138,6 +143,11 @@ export function renderDuration(entry: LogEntry): string {
 }
 
 export function formatForTerminal(entry: LogEntry): string {
+  const { msg, section, emoji, showDuration, symbol } = entry.opts
+  const empty = [msg, section, emoji, showDuration, symbol].every(val => val === undefined)
+  if (empty) {
+    return ""
+  }
   return combine([
     [leftPad, [entry]],
     [renderSymbol, [entry]],

garden-service/src/logger/util.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { LogNode, LogLevel } from "./log-node"
+import { LogNode } from "./log-node"
 import { LogEntry, CreateOpts } from "./log-entry"
 
 export interface Node {
@@ -105,7 +105,3 @@ export function getTerminalWidth(stream: NodeJS.WriteStream = process.stdout) {
 
   return columns
 }
-
-export function validate(level: LogLevel, entry: LogEntry): boolean {
-  return level >= entry.level && entry.opts.msg !== undefined
-}

garden-service/src/logger/writers/basic-terminal-writer.ts

@@ -10,15 +10,14 @@ import { LogLevel } from "../log-node"
 import { formatForTerminal } from "../renderers"
 import { LogEntry } from "../log-entry"
 import { Logger } from "../logger"
-import { validate } from "../util"
 import { Writer } from "./base"
 
 export class BasicTerminalWriter extends Writer {
   public level: LogLevel
 
   render(entry: LogEntry, logger: Logger): string | null {
     const level = this.level || logger.level
-    if (validate(level, entry)) {
+    if (level >= entry.level) {
       return formatForTerminal(entry)
     }
     return null

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

@@ -24,7 +24,6 @@ import {
   getChildEntries,
   getTerminalWidth,
   interceptStream,
-  validate,
 } from "../util"
 import { Writer, WriterConfig } from "./base"
 
@@ -191,7 +190,7 @@ export class FancyTerminalWriter extends Writer {
     let currentLineNumber = 0
 
     return getChildEntries(logger)
-      .filter(entry => validate(level, entry))
+      .filter(entry => level >= entry.level)
       .reduce((acc: TerminalEntry[], entry: LogEntry): TerminalEntry[] => {
         let spinnerFrame = ""
         let spinnerX
@@ -223,12 +222,14 @@ export class FancyTerminalWriter extends Writer {
           }))
           .pop()!
 
-        acc.push({
-          key: entry.key,
-          lineNumber: currentLineNumber,
-          spinnerCoords,
-          text,
-        })
+        if (text) {
+          acc.push({
+            key: entry.key,
+            lineNumber: currentLineNumber,
+            spinnerCoords,
+            text,
+          })
+        }
 
         currentLineNumber += text.split("\n").length - 1
 

garden-service/src/logger/writers/file-writer.ts

@@ -14,7 +14,6 @@ import { ensureDir, truncate } from "fs-extra"
 import { LogLevel } from "../log-node"
 import { LogEntry } from "../log-entry"
 import { Writer } from "./base"
-import { validate } from "../util"
 import {
   renderError,
   renderMsg,
@@ -45,6 +44,14 @@ const DEFAULT_FILE_TRANSPORT_OPTIONS: FileTransportOptions = {
 
 const levelToStr = (lvl: LogLevel): string => LogLevel[lvl]
 
+export function render(level: LogLevel, entry: LogEntry): string | null {
+  if (level >= entry.level) {
+    const renderFn = entry.level === LogLevel.error ? renderError : renderMsg
+    return stripAnsi(renderFn(entry))
+  }
+  return null
+}
+
 export class FileWriter extends Writer {
   private fileLogger: winston.Logger | null
   private filePath: string
@@ -98,11 +105,7 @@ export class FileWriter extends Writer {
   }
 
   render(entry: LogEntry): string | null {
-    if (validate(this.level, entry)) {
-      const renderFn = entry.level === LogLevel.error ? renderError : renderMsg
-      return stripAnsi(renderFn(entry))
-    }
-    return null
+    return render(this.level, entry)
   }
 
   onGraphChange(entry: LogEntry) {

garden-service/test/src/logger.ts

@@ -1,10 +1,20 @@
 import { expect } from "chai"
+import chalk from "chalk"
+import * as stripAnsi from "strip-ansi"
+
 import { LogLevel } from "../../src/logger/log-node"
 import { BasicTerminalWriter } from "../../src/logger/writers/basic-terminal-writer"
 import { FancyTerminalWriter } from "../../src/logger/writers/fancy-terminal-writer"
 import { getLogger } from "../../src/logger/logger"
 import { getChildNodes } from "../../src/logger/util"
-import { renderMsg, msgStyle, errorStyle } from "../../src/logger/renderers"
+import {
+  renderMsg,
+  msgStyle,
+  errorStyle,
+  formatForTerminal,
+  renderError,
+} from "../../src/logger/renderers"
+import { render } from "../../src/logger/writers/file-writer"
 
 const logger = getLogger()
 
@@ -88,74 +98,89 @@ describe("RootLogNode", () => {
 })
 
 describe("Writers", () => {
-  describe("BasicTerminalWriter.render", () => {
-    it("should return a string if level is geq than entry level and entry contains a message", () => {
-      const writer = new BasicTerminalWriter()
-      const entry = logger.info("")
-      const out = writer.render(entry, logger)
-      expect(out).to.eql("\n")
-    })
-    it("should override root level if level is set", () => {
-      const writer = new BasicTerminalWriter({ level: LogLevel.verbose })
-      const entry = logger.verbose("")
-      const out = writer.render(entry, logger)
-      expect(out).to.eql("\n")
-    })
-    it("should return null if entry level is geq to writer level", () => {
-      const writer = new BasicTerminalWriter()
-      const entry = logger.verbose("")
-      const out = writer.render(entry, logger)
-      expect(out).to.eql(null)
-    })
-    it("should return null if entry is empty", () => {
-      const writer = new BasicTerminalWriter()
-      const entry = logger.info()
-      const out = writer.render(entry, logger)
-      expect(out).to.eql(null)
-    })
-    it("should return null if entry has no message", () => {
-      const writer = new BasicTerminalWriter()
-      const entry = logger.info({})
-      const out = writer.render(entry, logger)
-      expect(out).to.eql(null)
+  describe("BasicTerminalWriter", () => {
+    describe("render", () => {
+      it("should return a string if level is geq than entry level and entry contains a message", () => {
+        const writer = new BasicTerminalWriter()
+        const entry = logger.info("")
+        const out = writer.render(entry, logger)
+        expect(out).to.eql("\n")
+      })
+      it("should override root level if level is set", () => {
+        const writer = new BasicTerminalWriter({ level: LogLevel.verbose })
+        const entry = logger.verbose("")
+        const out = writer.render(entry, logger)
+        expect(out).to.eql("\n")
+      })
+      it("should return null if entry level is geq to writer level", () => {
+        const writer = new BasicTerminalWriter()
+        const entry = logger.verbose("abc")
+        const out = writer.render(entry, logger)
+        expect(out).to.eql(null)
+      })
+      it("should return an empty string if entry is empty", () => {
+        const writer = new BasicTerminalWriter()
+        const entry = logger.info()
+        const out = writer.render(entry, logger)
+        expect(out).to.eql("")
+      })
     })
   })
 
-  describe("FancyTerminalWriter.toTerminalEntries", () => {
-    const writer = new FancyTerminalWriter()
-    const verboseWriter = new FancyTerminalWriter({ level: LogLevel.verbose })
-    writer.stop()
-    verboseWriter.stop()
-    it("should map a LogNode into an array of entries with line numbers and spinner positions", () => {
-      logger.info("1 line") // 0
-      logger.info("2 lines\n") // 1
-      logger.info("1 line") // 3
-      logger.info("3 lines\n\n") // 4
-      const spinner = logger.info({ msg: "spinner", status: "active" }) // 7
-      spinner.info({ msg: "nested spinner", status: "active" }) // 8
-      const terminalEntries = writer.toTerminalEntries(logger)
-      const lineNumbers = terminalEntries.map(e => e.lineNumber)
-      const spinners = terminalEntries.filter(e => !!e.spinnerCoords).map(e => e.spinnerCoords)
-      expect(lineNumbers).to.eql([0, 1, 3, 4, 7, 8])
-      expect(spinners).to.eql([[0, 7], [3, 8]])
-    })
-    it("should override root level if level is set", () => {
-      const entry = logger.verbose("")
-      const terminalEntries = verboseWriter.toTerminalEntries(logger)
-      expect(terminalEntries[0].key).to.eql(entry.key)
-    })
-    it("should skip entry if entry level is geq to writer level", () => {
-      logger.verbose("")
-      const terminalEntries = writer.toTerminalEntries(logger)
-      expect(terminalEntries).to.eql([])
-    })
-    it("should skip entry if entry is empty", () => {
-      logger.info()
-      const terminalEntries = writer.toTerminalEntries(logger)
-      expect(terminalEntries).to.eql([])
+  describe("FancyTerminalWriter", () => {
+    describe("toTerminalEntries", () => {
+      const writer = new FancyTerminalWriter()
+      const verboseWriter = new FancyTerminalWriter({ level: LogLevel.verbose })
+      writer.stop()
+      verboseWriter.stop()
+      it("should map a LogNode into an array of entries with line numbers and spinner positions", () => {
+        logger.info("1 line") // 0
+        logger.info("2 lines\n") // 1
+        logger.info("1 line") // 3
+        logger.info("3 lines\n\n") // 4
+        const spinner = logger.info({ msg: "spinner", status: "active" }) // 7
+        spinner.info({ msg: "nested spinner", status: "active" }) // 8
+        const terminalEntries = writer.toTerminalEntries(logger)
+        const lineNumbers = terminalEntries.map(e => e.lineNumber)
+        const spinners = terminalEntries.filter(e => !!e.spinnerCoords).map(e => e.spinnerCoords)
+        expect(lineNumbers).to.eql([0, 1, 3, 4, 7, 8])
+        expect(spinners).to.eql([[0, 7], [3, 8]])
+      })
+      it("should override root level if level is set", () => {
+        const entry = logger.verbose("")
+        const terminalEntries = verboseWriter.toTerminalEntries(logger)
+        expect(terminalEntries[0].key).to.eql(entry.key)
+      })
+      it("should skip entry if entry level is geq to writer level", () => {
+        logger.verbose("")
+        const terminalEntries = writer.toTerminalEntries(logger)
+        expect(terminalEntries).to.eql([])
+      })
+      it("should skip entry if entry is empty", () => {
+        logger.info()
+        const terminalEntries = writer.toTerminalEntries(logger)
+        expect(terminalEntries).to.eql([])
+      })
     })
   })
 
+  describe("FileWriter", () => {
+    describe("render", () => {
+      it("should render message without ansi characters", () => {
+        const entry = logger.info(chalk.red("hello"))
+        expect(render(LogLevel.info, entry)).to.equal("hello")
+      })
+      it("should render error message if entry level is error", () => {
+        const entry = logger.error("error")
+        const expectedOutput = stripAnsi(renderError(entry))
+        expect(render(LogLevel.info, entry)).to.equal(expectedOutput)
+      })
+      it("should return null if entry level is geq to writer level", () => {
+        const entry = logger.silly("silly")
+        expect(render(LogLevel.info, entry)).to.equal(null)
+      })
+    })
+  })
 })
 
 describe("LogEntry", () => {
@@ -251,6 +276,16 @@ describe("renderers", () => {
         expect(renderMsg(entry)).to.equal(errorStyle("error a") + errorStyle(" → ") + errorStyle("error b"))
       })
   })
+  describe("formatForTerminal", () => {
+    it("should return the entry as a formatted string with a new line character", () => {
+      const entry = logger.info("")
+      expect(formatForTerminal(entry)).to.equal("\n")
+    })
+    it("should return an empty string without a new line if the entry is empty", () => {
+      const entry = logger.info()
+      expect(formatForTerminal(entry)).to.equal("")
+    })
+  })
 })
 
 describe("util", () => {