Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions garden-cli/src/logger/log-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface UpdateOpts {
showDuration?: boolean
error?: GardenError
status?: EntryStatus
indentationLevel?: number
}

export interface CreateOpts extends UpdateOpts {
Expand All @@ -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 || {}
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
15 changes: 7 additions & 8 deletions garden-cli/src/logger/log-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) {
Expand All @@ -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)
}

/**
Expand Down
8 changes: 3 additions & 5 deletions garden-cli/src/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down Expand Up @@ -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) {
Expand Down
33 changes: 10 additions & 23 deletions garden-cli/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"],
])
}
17 changes: 11 additions & 6 deletions garden-cli/src/logger/writers/fancy-terminal-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
)
}

Expand Down Expand Up @@ -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) {
Expand All @@ -152,7 +157,7 @@ export class FancyTerminalWriter extends Writer {
if (this.updatePending) {
this.handleGraphChange(logEntry, logger, true)
}
}, FANCY_LOGGER_THROTTLE_MS)
}, THROTTLE_MS)
return
}
}
Expand Down
71 changes: 46 additions & 25 deletions garden-cli/test/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down