Skip to content

Commit

Permalink
improvement(core): do not truncate logger sections
Browse files Browse the repository at this point in the history
Also fixes some lint and test errors that accidentally got merged to
master.
  • Loading branch information
eysi09 authored and edvald committed Aug 5, 2021
1 parent ba66484 commit f462e3a
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 92 deletions.
6 changes: 3 additions & 3 deletions core/src/commands/logs.ts
Expand Up @@ -19,7 +19,7 @@ import { printHeader, renderDivider } from "../logger/util"
import stripAnsi = require("strip-ansi")
import hasAnsi = require("has-ansi")
import { dedent } from "../util/string"
import { formatSection } from "../logger/renderers"
import { padSection } from "../logger/renderers"

const logsArgs = {
services: new StringsParameter({
Expand Down Expand Up @@ -187,10 +187,10 @@ export class LogsCommand extends Command<Args, Opts> {

let out = ""
if (!hideService) {
out += `${sectionStyle(formatSection(entry.serviceName, maxServiceName))} → `
out += `${sectionStyle(padSection(entry.serviceName, maxServiceName))} → `
}
if (container) {
out += `${sectionStyle(formatSection(container, maxContainerName))} → `
out += `${sectionStyle(padSection(container, maxContainerName))} → `
}
if (timestamp) {
out += `${chalk.gray(timestamp)} → `
Expand Down
7 changes: 6 additions & 1 deletion core/src/exceptions.ts
Expand Up @@ -25,14 +25,19 @@ export abstract class GardenBaseError extends Error implements GardenError {
}
}

interface ErrorEvent {
error: any
message: string
}

export function toGardenError(err: Error | ErrorEvent | GardenBaseError | string): GardenBaseError {
if (err instanceof GardenBaseError) {
return err
} else if (err instanceof Error) {
const out = new RuntimeError(err.message, err)
out.stack = err.stack
return out
} else if (err instanceof ErrorEvent) {
} else if (!isString(err) && err.message && err.error) {
return new RuntimeError(err.message, err)
} else if (isString(err)) {
return new RuntimeError(err, {})
Expand Down
4 changes: 0 additions & 4 deletions core/src/logger/log-entry.ts
Expand Up @@ -49,7 +49,6 @@ interface MessageBase {
append?: boolean
data?: any
dataFormat?: "json" | "yaml"
maxSectionWidth?: number
}

export interface LogEntryMessage extends MessageBase {
Expand Down Expand Up @@ -136,7 +135,6 @@ export class LogEntry implements LogNode {
data: params.data,
dataFormat: params.dataFormat,
append: params.append,
maxSectionWidth: params.maxSectionWidth,
})
} else {
this.messages = [{ timestamp: new Date() }]
Expand Down Expand Up @@ -166,8 +164,6 @@ export class LogEntry implements LogNode {
// Next message does not inherit the append field
append: updateParams.append,
timestamp: new Date(),
maxSectionWidth:
updateParams.maxSectionWidth !== undefined ? updateParams.maxSectionWidth : latestMessage.maxSectionWidth,
}

// Hack to preserve section alignment if spinner disappears
Expand Down
22 changes: 7 additions & 15 deletions core/src/logger/renderers.ts
Expand Up @@ -10,7 +10,6 @@ import logSymbols from "log-symbols"
import chalk from "chalk"
import stripAnsi from "strip-ansi"
import { isArray, repeat } from "lodash"
import cliTruncate = require("cli-truncate")
import stringWidth = require("string-width")
import hasAnsi = require("has-ansi")

Expand All @@ -24,18 +23,11 @@ type RenderFn = (entry: LogEntry) => string

/*** STYLE HELPERS ***/

export const MAX_SECTION_WIDTH = 25
const cliPadEnd = (s: string, width: number): string => {
const diff = width - stringWidth(s)
return diff <= 0 ? s : s + repeat(" ", diff)
}
export const SECTION_PADDING = 25

export function formatSection(section: string, width: number = MAX_SECTION_WIDTH) {
const minWidth = Math.min(width, MAX_SECTION_WIDTH)
return [section]
.map((s) => cliTruncate(s, minWidth))
.map((s) => cliPadEnd(s, minWidth))
.pop()
export function padSection(section: string, width: number = SECTION_PADDING) {
const diff = width - stringWidth(section)
return diff <= 0 ? section : section + repeat(" ", diff)
}

export const msgStyle = (s: string) => (hasAnsi(s) ? s : chalk.gray(s))
Expand Down Expand Up @@ -166,11 +158,11 @@ export function renderData(entry: LogEntry): string {

export function renderSection(entry: LogEntry): string {
const style = chalk.cyan.italic
const { msg: msg, section, maxSectionWidth } = entry.getLatestMessage()
const { msg: msg, section } = entry.getLatestMessage()
if (section && msg) {
return `${style(formatSection(section, maxSectionWidth))} → `
return `${style(padSection(section))} → `
} else if (section) {
return style(formatSection(section, maxSectionWidth))
return style(padSection(section))
}
return ""
}
Expand Down
52 changes: 0 additions & 52 deletions core/test/unit/src/logger/log-entry.ts
Expand Up @@ -29,7 +29,6 @@ describe("LogEntry", () => {
data: undefined,
dataFormat: undefined,
append: undefined,
maxSectionWidth: undefined,
}
it("should create log entries with the appropriate fields set", () => {
const timestamp = freezeTime()
Expand All @@ -43,7 +42,6 @@ describe("LogEntry", () => {
append: true,
data: { foo: "bar" },
dataFormat: "json",
maxSectionWidth: 20,
metadata: {
workflowStep: {
index: 2,
Expand All @@ -65,7 +63,6 @@ describe("LogEntry", () => {
append: true,
data: { foo: "bar" },
dataFormat: "json",
maxSectionWidth: 20,
timestamp,
},
])
Expand Down Expand Up @@ -189,7 +186,6 @@ describe("LogEntry", () => {
data: { some: "data" },
dataFormat: "json",
metadata: { task: taskMetadata },
maxSectionWidth: 8,
})

expect(entry.getMessages()).to.eql([
Expand All @@ -203,39 +199,10 @@ describe("LogEntry", () => {
dataFormat: "json",
append: undefined,
timestamp,
maxSectionWidth: 8,
},
])
expect(entry.getMetadata()).to.eql({ task: taskMetadata })
})
it("should update maxSectionWidth to zero", () => {
const timestamp = freezeTime()
const entry = logger.placeholder()
entry.setState({
msg: "hello",
emoji: "haircut",
section: "caesar",
symbol: "info",
status: "done",
data: { some: "data" },
maxSectionWidth: 0,
})

expect(entry.getMessages()).to.eql([
{
msg: "hello",
emoji: "haircut",
section: "caesar",
symbol: "info",
status: "done",
data: { some: "data" },
dataFormat: undefined,
append: undefined,
timestamp,
maxSectionWidth: 0,
},
])
})
it("should overwrite previous values", () => {
const timestamp = freezeTime()
const entry = logger.placeholder()
Expand All @@ -246,18 +213,13 @@ describe("LogEntry", () => {
symbol: "info",
status: "done",
data: { some: "data" },
maxSectionWidth: 8,
})
entry.setState({
msg: "world",
emoji: "hamburger",
data: { some: "data_updated" },
maxSectionWidth: 10,
})

entry.setState({
maxSectionWidth: 0,
})
expect(entry.getMessages()).to.eql([
{
msg: "hello",
Expand All @@ -269,19 +231,6 @@ describe("LogEntry", () => {
dataFormat: undefined,
append: undefined,
timestamp,
maxSectionWidth: 8,
},
{
msg: "world",
emoji: "hamburger",
section: "caesar",
symbol: "info",
status: "done",
data: { some: "data_updated" },
dataFormat: undefined,
append: undefined,
timestamp,
maxSectionWidth: 10,
},
{
msg: "world",
Expand All @@ -293,7 +242,6 @@ describe("LogEntry", () => {
dataFormat: undefined,
append: undefined,
timestamp,
maxSectionWidth: 0,
},
])
})
Expand Down
3 changes: 0 additions & 3 deletions core/test/unit/src/logger/logger.ts
Expand Up @@ -43,7 +43,6 @@ describe("Logger", () => {
append: true,
data: { foo: "bar" },
dataFormat: "json",
maxSectionWidth: 20,
metadata: {
workflowStep: {
index: 2,
Expand All @@ -67,7 +66,6 @@ describe("Logger", () => {
append: true,
dataFormat: "json",
data: { foo: "bar" },
maxSectionWidth: 20,
},
metadata: {
workflowStep: {
Expand All @@ -88,7 +86,6 @@ describe("Logger", () => {
append: undefined,
dataFormat: undefined,
data: undefined,
maxSectionWidth: undefined,
}

const [e1, e2] = loggerEvents
Expand Down
21 changes: 7 additions & 14 deletions core/test/unit/src/logger/renderers.ts
Expand Up @@ -18,7 +18,7 @@ import {
renderError,
formatForJson,
renderSection,
MAX_SECTION_WIDTH,
SECTION_PADDING,
renderData,
} from "../../../../src/logger/renderers"
import { GardenError } from "../../../../src/exceptions"
Expand Down Expand Up @@ -100,29 +100,22 @@ describe("renderers", () => {
})
})
describe("renderSection", () => {
it("should render the log entry section", () => {
it("should render the log entry section with padding", () => {
const entry = logger.info({ msg: "foo", section: "hello" })
const withWhitespace = "hello".padEnd(MAX_SECTION_WIDTH, " ")
const withWhitespace = "hello".padEnd(SECTION_PADDING, " ")
const rendered = stripAnsi(renderSection(entry))
expect(rendered).to.equal(`${withWhitespace} → `)
})
it("should not render arrow if message is empty", () => {
const entry = logger.info({ section: "hello" })
const withWhitespace = "hello".padEnd(MAX_SECTION_WIDTH, " ")
const withWhitespace = "hello".padEnd(SECTION_PADDING, " ")
const rendered = stripAnsi(renderSection(entry))
expect(rendered).to.equal(`${withWhitespace}`)
})
it("should optionally set a custom section width", () => {
const entry = logger.info({ msg: "foo", section: "hello", maxSectionWidth: 8 })
const withWhitespace = "hello".padEnd(8, " ")
it("should not not truncate the section", () => {
const entry = logger.info({ msg: "foo", section: "very-very-very-very-very-long" })
const rendered = stripAnsi(renderSection(entry))
expect(rendered).to.equal(`${withWhitespace} → `)
})
it("should not let custom section width exceed max section width", () => {
const entry = logger.info({ msg: "foo", section: "hello", maxSectionWidth: 99 })
const withWhitespace = "hello".padEnd(MAX_SECTION_WIDTH, " ")
const rendered = stripAnsi(renderSection(entry))
expect(rendered).to.equal(`${withWhitespace} → `)
expect(rendered).to.equal(`very-very-very-very-very-long → `)
})
})
describe("chainMessages", () => {
Expand Down

0 comments on commit f462e3a

Please sign in to comment.