From 9c428cda2088c71ab8ee268dcce87eecafdf7c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Tue, 25 Sep 2018 11:33:11 +0000 Subject: [PATCH] fix(logger): let empty entries inherit parent indentation level --- garden-cli/src/logger/log-entry.ts | 30 ++++---- garden-cli/src/logger/log-node.ts | 15 ++-- garden-cli/src/logger/logger.ts | 8 +-- garden-cli/src/logger/renderers.ts | 33 +++------ .../logger/writers/fancy-terminal-writer.ts | 17 +++-- garden-cli/test/src/logger.ts | 71 ++++++++++++------- 6 files changed, 92 insertions(+), 82 deletions(-) diff --git a/garden-cli/src/logger/log-entry.ts b/garden-cli/src/logger/log-entry.ts index b03d761905..83405cb33d 100644 --- a/garden-cli/src/logger/log-entry.ts +++ b/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(param?: string | T): T { return typeof param === "string" ? { msg: param } : param || {} @@ -62,9 +52,9 @@ export function resolveParam(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 } diff --git a/garden-cli/src/logger/log-node.ts b/garden-cli/src/logger/log-node.ts index 4e9e0b3d0f..21a06db91b 100644 --- a/garden-cli/src/logger/log-node.ts +++ b/garden-cli/src/logger/log-node.ts @@ -29,7 +29,6 @@ export abstract class LogNode { constructor( public readonly level: LogLevel, - public readonly depth: number, public readonly parent?: LogNode, public readonly id?: string, ) { @@ -46,7 +45,7 @@ export abstract class LogNode { abstract createNode(level: LogLevel, parent: LogNode, 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) @@ -54,27 +53,27 @@ export abstract class LogNode { } 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) } /** diff --git a/garden-cli/src/logger/logger.ts b/garden-cli/src/logger/logger.ts index 8b7d0fe4a7..4360c3cdbd 100644 --- a/garden-cli/src/logger/logger.ts +++ b/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 { } 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) { diff --git a/garden-cli/src/logger/renderers.ts b/garden-cli/src/logger/renderers.ts index 90a3ff7c2e..a8f04a3e06 100644 --- a/garden-cli/src/logger/renderers.ts +++ b/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"], + ]) } diff --git a/garden-cli/src/logger/writers/fancy-terminal-writer.ts b/garden-cli/src/logger/writers/fancy-terminal-writer.ts index 922b8074a4..180250ac75 100644 --- a/garden-cli/src/logger/writers/fancy-terminal-writer.ts +++ b/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 } } diff --git a/garden-cli/test/src/logger.ts b/garden-cli/test/src/logger.ts index 08a356a940..07fa142853 100644 --- a/garden-cli/test/src/logger.ts +++ b/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", () => {