From 1e8e5cf112752e7d08c69b0cdc525aacce6b853b Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Wed, 24 Nov 2021 16:04:12 -0700 Subject: [PATCH] resolves #884 use more modest colorization of prettified log message; move hint to hint key on log object --- CHANGELOG.adoc | 2 ++ packages/cli/lib/cli.js | 2 +- packages/cli/test/cli-test.js | 5 ++- packages/logger/lib/logger.js | 8 +++++ packages/logger/test/logger-test.js | 51 +++++++++++++++++++++++++---- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 39010d0e9..0c89c634c 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -15,6 +15,8 @@ This project utilizes semantic versioning. * *logger*: Move error to `err` property on structured log object; deduplicate information (#878) * *logger*: Move all error formatting from CLI to logger (#878) +* *logger*: Use more modest colorization of prettified log message; only colorize first line; colorize hint as dim neutral (#884) +* *logger*: Set hint on hint key of log object instead of appending it to the bottom of the message (#884) * *playbook-builder*: Include path of playbook file in message of any error thrown by playbook builder (#882) * *content-aggregator*: Replace vinyl-fs.src with glob-stream in a stream.pipeline (#839) * *ui-loader*: Replace vinyl-fs.src with glob-stream in a stream.pipeline (#839) diff --git a/packages/cli/lib/cli.js b/packages/cli/lib/cli.js index f572b729e..f98f857c6 100644 --- a/packages/cli/lib/cli.js +++ b/packages/cli/lib/cli.js @@ -27,7 +27,7 @@ function exitWithError (err, opts, msg = undefined) { if (opts.stacktrace) { getLogger(name).fatal(err, msg) } else { - getLogger(name).fatal(msg + '\nAdd the --stacktrace option to see the cause of the error.') + getLogger(name).fatal({ hint: 'Add the --stacktrace option to see the cause of the error.' }, msg) } return exit() } diff --git a/packages/cli/test/cli-test.js b/packages/cli/test/cli-test.js index a1e358a12..53e891667 100644 --- a/packages/cli/test/cli-test.js +++ b/packages/cli/test/cli-test.js @@ -1063,9 +1063,8 @@ describe('cli', function () { .to.be.a.file() //.with.json() // .with.json() doesn't understand the json lines format .and.have.contents.that.match(/"msg":"Start page specified for site not found: .+"/) - .and.have.contents.that.match( - /"msg":"Unsupported destination provider: s3\\nAdd the --stacktrace option to see the cause of the error\."/ - ) + .and.have.contents.that.match(/"msg":"Unsupported destination provider: s3"/) + .and.have.contents.that.match(/"hint":"Add the --stacktrace option to see the cause of the error\."/) }) }) diff --git a/packages/logger/lib/logger.js b/packages/logger/lib/logger.js index cb34290a6..ba9f15f47 100644 --- a/packages/logger/lib/logger.js +++ b/packages/logger/lib/logger.js @@ -148,6 +148,14 @@ function createPrettyDestination (destination, colorize) { ? `${worktree} (refname: ${refname} ${startPath ? ', start path: ' + startPath : ''})` : `${url || ''} (refname: ${refname}${startPath ? ', start path: ' + startPath : ''})`, }, + ignore: 'hint', + messageFormat: (log, msgKey) => { + let hint, msg + if (typeof (msg = log[msgKey]) !== 'string') return + if ((hint = log.hint)) msg += '\n' + (colorize ? `\x1b[2m${hint}\x1b[22m` : hint) + if (colorize) msg = msg.replace('\n', '\n\x1b[0m') + return msg + }, translateTime: 'SYS:HH:MM:ss.l', // Q: do we really need ms? should we honor DATE_FORMAT env var? }) } diff --git a/packages/logger/test/logger-test.js b/packages/logger/test/logger-test.js index dea7c2d54..181a7be0b 100644 --- a/packages/logger/test/logger-test.js +++ b/packages/logger/test/logger-test.js @@ -109,6 +109,13 @@ describe('logger', () => { expect(message).to.eql({ level: 'info', msg: 'love is the message' }) }) + it('should include hint key if specified in merging object', () => { + const logger = configure().get(null) + const lines = captureStdoutSync(() => logger.info({ hint: 'let the music play' }, 'love is the message')) + expect(lines).to.have.lengthOf(1) + expect(JSON.parse(lines[0])).to.include({ hint: 'let the music play', msg: 'love is the message' }) + }) + it('should allow fatal message to be logged', () => { const logger = configure().get(null) const lines = captureStdoutSync(() => logger.fatal("You've sunk my battleship!")) @@ -494,25 +501,57 @@ describe('logger', () => { }) if (supportsColor) { - it('should colorize pretty log message if supported by environment', () => { - delete process.env.NO_COLOR + it('should not colorize pretty log message if NO_COLOR environment variable is set', () => { + process.env.NO_COLOR = '1' const logger = configure({ name: 'antora', format: 'pretty' }).get() const lines = captureStderrSync(() => logger.info('love is the message')) expect(lines).to.have.lengthOf(1) - const expectedLine = '\u001b[32mINFO\u001b[39m (antora): \u001b[36mlove is the message\u001b[39m' + const expectedLine = 'INFO (antora): love is the message' expect(lines[0]).to.include(expectedLine) }) - it('should not colorize pretty log message if NO_COLOR environment variable is set', () => { - process.env.NO_COLOR = '1' + it('should colorize pretty log message if supported by environment', () => { + delete process.env.NO_COLOR const logger = configure({ name: 'antora', format: 'pretty' }).get() const lines = captureStderrSync(() => logger.info('love is the message')) expect(lines).to.have.lengthOf(1) - const expectedLine = 'INFO (antora): love is the message' + const expectedLine = '\u001b[32mINFO\u001b[39m (antora): \u001b[36mlove is the message\u001b[39m' expect(lines[0]).to.include(expectedLine) }) + + it('should colorize only first line of pretty log message if supported by environment', () => { + delete process.env.NO_COLOR + const logger = configure({ name: 'antora', format: 'pretty' }).get() + const lines = captureStderrSync(() => logger.info('love is the message\nmusic is the answer')) + expect(lines).to.have.lengthOf(2) + const expectedLine1 = '\u001b[32mINFO\u001b[39m (antora): \u001b[36mlove is the message' + const expectedLine2 = '\u001b[0mmusic is the answer\u001b[39m' + expect(lines[0]).to.include(expectedLine1) + expect(lines[1]).to.include(expectedLine2) + }) + + it('should colorize hint of pretty log message if supported by environment', () => { + delete process.env.NO_COLOR + const logger = configure({ name: 'antora', format: 'pretty' }).get() + const lines = captureStderrSync(() => logger.info({ hint: 'let the music play' }, 'love is the message')) + expect(lines).to.have.lengthOf(2) + const expectedLine1 = '\u001b[32mINFO\u001b[39m (antora): \u001b[36mlove is the message' + const expectedLine2 = '\u001b[0m\u001b[2mlet the music play\u001b[22m' + expect(lines[0]).to.include(expectedLine1) + expect(lines[1]).to.include(expectedLine2) + }) } + it('should append hint in merging object below message', () => { + const logger = configure({ name: 'antora', format: 'pretty' }).get() + const lines = captureStderrSync(() => logger.info({ hint: 'let the music play' }, 'love is the message')) + expect(lines).to.have.lengthOf(2) + const expectedLine1 = 'INFO (antora): love is the message' + const expectedLine2 = 'let the music play' + expect(lines[0]).to.include(expectedLine1) + expect(lines[1]).to.include(expectedLine2) + }) + it('should not log warning that flushSync is not supported when fatal message is logged', () => { const logger = configure({ format: 'pretty' }).get() const lines = captureStderrSync(() => logger.fatal("You've sunk my battleship!")) -- GitLab