From 3ed58ceea771ca14d81b85715ef0ff397fca4880 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 28 Apr 2023 16:31:29 -0700 Subject: [PATCH 1/6] chore(ts): small tweaks to make typescript happier - Use trimStart/trimEnd instead of deprecated trimLeft/trimRight --- lib/index.ts | 6 +++--- tsconfig.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index 0673c3a..2188e50 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -204,7 +204,7 @@ export class UI { const match = source.match(/^ */) const leadingWhitespace = match ? match[0].length : 0 const target = previousLine.text - const targetTextWidth = mixin.stringWidth(target.trimRight()) + const targetTextWidth = mixin.stringWidth(target.trimEnd()) if (!previousLine.span) { return source @@ -223,13 +223,13 @@ export class UI { previousLine.hidden = true - return target.trimRight() + ' '.repeat(leadingWhitespace - targetTextWidth) + source.trimLeft() + return target.trimEnd() + ' '.repeat(leadingWhitespace - targetTextWidth) + source.trimStart() } private rasterize (row: ColumnArray) { const rrows: string[][] = [] const widths = this.columnWidths(row) - let wrapped + let wrapped: string[] // word wrap all columns, and create // a data-structure that is easy to rasterize. diff --git a/tsconfig.json b/tsconfig.json index d5d8c21..5b52b82 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,11 +7,11 @@ "target": "es2017", "moduleResolution": "node", "module": "es2015" - }, + }, "include": [ "lib/**/*.ts" ], "exclude": [ "lib/cjs.ts" ] -} \ No newline at end of file +} From 7a37884fd2b3bd4b36ca19db68c587a112b4f8f6 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 29 Apr 2023 12:54:26 -0700 Subject: [PATCH 2/6] fix: handle strings the same in cjs, esm, and deno This also ports some of the `// istanbul ignore` comments to their associated `/* c8 ignore start/stop */` equivalents, and coverage-ignores some value fallbacks that are there for safety but can never be hit in normal usage. Fix: #138 --- deno.ts | 11 ++++++----- index.mjs | 11 ++++++----- lib/cjs.ts | 6 +++--- lib/index.ts | 19 ++++++++++++++++++- lib/string-utils.ts | 30 ------------------------------ package.json | 9 ++++++--- test/cjs-esm-compare.cjs | 34 ++++++++++++++++++++++++++++++++++ test/cliui.cjs | 2 +- 8 files changed, 74 insertions(+), 48 deletions(-) delete mode 100644 lib/string-utils.ts create mode 100644 test/cjs-esm-compare.cjs diff --git a/deno.ts b/deno.ts index a94e49f..5442f95 100644 --- a/deno.ts +++ b/deno.ts @@ -1,13 +1,14 @@ -// Bootstrap cliui with CommonJS dependencies: +// Bootstrap cliui with ESM dependencies in Deno's style: import { cliui, UI } from './build/lib/index.js' import type { UIOptions } from './build/lib/index.d.ts' -import { wrap, stripAnsi } from './build/lib/string-utils.js' + +import stringWidth from 'npm:string-width' +import stripAnsi from 'npm:strip-ansi' +import wrap from 'npm:wrap-ansi' export default function ui (opts: UIOptions): UI { return cliui(opts, { - stringWidth: (str: string) => { - return [...str].length - }, + stringWidth, stripAnsi, wrap }) diff --git a/index.mjs b/index.mjs index bc7a022..5177519 100644 --- a/index.mjs +++ b/index.mjs @@ -1,12 +1,13 @@ -// Bootstrap cliui with CommonJS dependencies: +// Bootstrap cliui with ESM dependencies: import { cliui } from './build/lib/index.js' -import { wrap, stripAnsi } from './build/lib/string-utils.js' + +import stringWidth from 'string-width' +import stripAnsi from 'strip-ansi' +import wrap from 'wrap-ansi' export default function ui (opts) { return cliui(opts, { - stringWidth: (str) => { - return [...str].length - }, + stringWidth, stripAnsi, wrap }) diff --git a/lib/cjs.ts b/lib/cjs.ts index bda4241..32b8087 100644 --- a/lib/cjs.ts +++ b/lib/cjs.ts @@ -1,8 +1,8 @@ // Bootstrap cliui with CommonJS dependencies: import { cliui, UIOptions } from './index.js' -const stringWidth = require('string-width') -const stripAnsi = require('strip-ansi') -const wrap = require('wrap-ansi') +const stringWidth = require('string-width-cjs') +const stripAnsi = require('strip-ansi-cjs') +const wrap = require('wrap-ansi-cjs') export default function ui (opts: UIOptions) { return cliui(opts, { stringWidth, diff --git a/lib/index.ts b/lib/index.ts index 2188e50..0fbb160 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -47,7 +47,9 @@ export class UI { constructor (opts: UIOptions) { this.width = opts.width + /* c8 ignore start */ this.wrap = opts.wrap ?? true + /* c8 ignore stop */ this.rows = [] } @@ -164,7 +166,10 @@ export class UI { const fn = align[(row[c].align as 'right'|'center')] ts = fn(ts, wrapWidth) if (mixin.stringWidth(ts) < wrapWidth) { - ts += ' '.repeat((width || 0) - mixin.stringWidth(ts) - 1) + /* c8 ignore start */ + const w = width || 0 + /* c8 ignore stop */ + ts += ' '.repeat(w - mixin.stringWidth(ts) - 1) } } @@ -202,7 +207,9 @@ export class UI { // the target line, do so. private renderInline (source: string, previousLine: Line) { const match = source.match(/^ */) + /* c8 ignore start */ const leadingWhitespace = match ? match[0].length : 0 + /* c8 ignore stop */ const target = previousLine.text const targetTextWidth = mixin.stringWidth(target.trimEnd()) @@ -274,7 +281,9 @@ export class UI { } private negatePadding (col: Column) { + /* c8 ignore start */ let wrapWidth = col.width || 0 + /* c8 ignore stop */ if (col.padding) { wrapWidth -= (col.padding[left] || 0) + (col.padding[right] || 0) } @@ -308,7 +317,9 @@ export class UI { }) // any unset widths should be calculated. + /* c8 ignore start */ const unsetWidth = unset ? Math.floor(remainingWidth / unset) : 0 + /* c8 ignore stop */ return widths.map((w, i) => { if (w === undefined) { @@ -349,12 +360,14 @@ function _minWidth (col: Column) { } function getWindowWidth (): number { + /* c8 ignore start */ /* istanbul ignore next: depends on terminal */ if (typeof process === 'object' && process.stdout && process.stdout.columns) { return process.stdout.columns } return 80 } +/* c8 ignore stop */ function alignRight (str: string, width: number): string { str = str.trim() @@ -371,10 +384,12 @@ function alignCenter (str: string, width: number): string { str = str.trim() const strWidth = mixin.stringWidth(str) + /* c8 ignore start */ /* istanbul ignore next */ if (strWidth >= width) { return str } + /* c8 ignore stop */ return ' '.repeat((width - strWidth) >> 1) + str } @@ -383,7 +398,9 @@ let mixin: Mixin export function cliui (opts: Partial, _mixin: Mixin) { mixin = _mixin return new UI({ + /* c8 ignore start */ width: opts?.width || getWindowWidth(), wrap: opts?.wrap + /* c8 ignore stop */ }) } diff --git a/lib/string-utils.ts b/lib/string-utils.ts deleted file mode 100644 index 23d78fd..0000000 --- a/lib/string-utils.ts +++ /dev/null @@ -1,30 +0,0 @@ -// Minimal replacement for ansi string helpers "wrap-ansi" and "strip-ansi". -// to facilitate ESM and Deno modules. -// TODO: look at porting https://www.npmjs.com/package/wrap-ansi to ESM. - -// The npm application -// Copyright (c) npm, Inc. and Contributors -// Licensed on the terms of The Artistic License 2.0 -// See: https://github.com/npm/cli/blob/4c65cd952bc8627811735bea76b9b110cc4fc80e/lib/utils/ansi-trim.js -const ansi = new RegExp('\x1b(?:\\[(?:\\d+[ABCDEFGJKSTm]|\\d+;\\d+[Hfm]|' + -'\\d+;\\d+;\\d+m|6n|s|u|\\?25[lh])|\\w)', 'g') - -export function stripAnsi (str: string) { - return str.replace(ansi, '') -} - -export function wrap (str: string, width: number) { - const [start, end] = str.match(ansi) || ['', ''] - str = stripAnsi(str) - let wrapped = '' - for (let i = 0; i < str.length; i++) { - if (i !== 0 && (i % width) === 0) { - wrapped += '\n' - } - wrapped += str.charAt(i) - } - if (start && end) { - wrapped = `${start}${wrapped}${end}` - } - return wrapped -} diff --git a/package.json b/package.json index eab6bf4..a5e76b7 100644 --- a/package.json +++ b/package.json @@ -49,9 +49,12 @@ "author": "Ben Coe ", "license": "ISC", "dependencies": { - "string-width": "^4.2.0", - "strip-ansi": "^6.0.1", - "wrap-ansi": "^7.0.0" + "string-width": "^5.1.2", + "string-width-cjs": "npm:string-width@^4.2.0", + "strip-ansi": "^7.0.1", + "strip-ansi-cjs": "npm:strip-ansi@^6.0.1", + "wrap-ansi": "^8.1.0", + "wrap-ansi-cjs": "npm:wrap-ansi@^7.0.0" }, "devDependencies": { "@types/node": "^14.0.27", diff --git a/test/cjs-esm-compare.cjs b/test/cjs-esm-compare.cjs new file mode 100644 index 0000000..d42506f --- /dev/null +++ b/test/cjs-esm-compare.cjs @@ -0,0 +1,34 @@ +'use strict' + +/* global describe, it */ + +require('chai').should() + +const text = `usage: git tag [-a | -s | -u ] [-f] [-m | -F ] [-e] + [ | ] + or: git tag -d ... + or: git tag [-n[]] -l [--contains ] [--no-contains ] + [--points-at ] [--column[=] | --no-column] + [--create-reflog] [--sort=] [--format=] + [--merged ] [--no-merged ] [...] + or: git tag -v [--format=] ...` + + +const cliuiCJS = require('../build/index.cjs') +import('../index.mjs').then(({ default: cliuiESM }) => { + describe('consistent wrapping', () => { + it('should produce matching output in cjs and esm', () => { + const uiCJS = cliuiCJS({}) + const uiESM = cliuiESM({}) + uiCJS.div({ + padding: [0, 0, 0, 0], + text, + }) + uiESM.div({ + padding: [0, 0, 0, 0], + text, + }) + uiCJS.toString().should.equal(uiESM.toString()) + }) + }) +}) diff --git a/test/cliui.cjs b/test/cliui.cjs index 6dd86e7..d2edfac 100644 --- a/test/cliui.cjs +++ b/test/cliui.cjs @@ -9,7 +9,7 @@ process.env.FORCE_COLOR = 1 const chalk = require('chalk') const cliui = require('../build/index.cjs') -const stripAnsi = require('strip-ansi') +const stripAnsi = require('strip-ansi-cjs') describe('cliui', () => { describe('resetOutput', () => { From 8b610456dfa3a52678dafa138066675c58f292f9 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 29 Apr 2023 22:16:05 -0700 Subject: [PATCH 3/6] chore: remove istanbul comments --- lib/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index 0fbb160..9f51738 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -361,7 +361,6 @@ function _minWidth (col: Column) { function getWindowWidth (): number { /* c8 ignore start */ - /* istanbul ignore next: depends on terminal */ if (typeof process === 'object' && process.stdout && process.stdout.columns) { return process.stdout.columns } @@ -385,7 +384,6 @@ function alignCenter (str: string, width: number): string { const strWidth = mixin.stringWidth(str) /* c8 ignore start */ - /* istanbul ignore next */ if (strWidth >= width) { return str } From fafee28a1e36415519023c551591514780a62c69 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sun, 30 Apr 2023 19:36:37 -0700 Subject: [PATCH 4/6] chore: move cjs to mjs so standardx doesn't check it --- ...js-esm-compare.cjs => cjs-esm-compare.mjs} | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) rename test/{cjs-esm-compare.cjs => cjs-esm-compare.mjs} (58%) diff --git a/test/cjs-esm-compare.cjs b/test/cjs-esm-compare.mjs similarity index 58% rename from test/cjs-esm-compare.cjs rename to test/cjs-esm-compare.mjs index d42506f..090db95 100644 --- a/test/cjs-esm-compare.cjs +++ b/test/cjs-esm-compare.mjs @@ -2,6 +2,8 @@ /* global describe, it */ +import { createRequire } from 'module' +const require = createRequire(import.meta.url) require('chai').should() const text = `usage: git tag [-a | -s | -u ] [-f] [-m | -F ] [-e] @@ -15,20 +17,19 @@ const text = `usage: git tag [-a | -s | -u ] [-f] [-m | -F ] const cliuiCJS = require('../build/index.cjs') -import('../index.mjs').then(({ default: cliuiESM }) => { - describe('consistent wrapping', () => { - it('should produce matching output in cjs and esm', () => { - const uiCJS = cliuiCJS({}) - const uiESM = cliuiESM({}) - uiCJS.div({ - padding: [0, 0, 0, 0], - text, - }) - uiESM.div({ - padding: [0, 0, 0, 0], - text, - }) - uiCJS.toString().should.equal(uiESM.toString()) +import cliuiESM from '../index.mjs' +describe('consistent wrapping', () => { + it('should produce matching output in cjs and esm', () => { + const uiCJS = cliuiCJS({}) + const uiESM = cliuiESM({}) + uiCJS.div({ + padding: [0, 0, 0, 0], + text, }) + uiESM.div({ + padding: [0, 0, 0, 0], + text, + }) + uiCJS.toString().should.equal(uiESM.toString()) }) }) From f2366630a04255a3bf336f89cf447493ea52e04b Mon Sep 17 00:00:00 2001 From: isaacs Date: Sun, 30 Apr 2023 19:40:56 -0700 Subject: [PATCH 5/6] chore(test): update esm test to verify correct behavior Previously, it was verifying *incorrect* behavior. --- package.json | 2 +- test/deno/cliui-test.ts | 11 +++++------ test/esm/cliui-test.mjs | 11 ++++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index a5e76b7..c126eee 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "fix": "standardx --fix '**/*.ts' && standardx --fix '**/*.js' && standardx --fix '**/*.cjs'", "pretest": "rimraf build && tsc -p tsconfig.test.json && cross-env NODE_ENV=test npm run build:cjs", "test": "c8 mocha ./test/*.cjs", - "test:esm": "c8 mocha ./test/esm/cliui-test.mjs", + "test:esm": "c8 mocha ./test/**/*.mjs", "postest": "check", "coverage": "c8 report --check-coverage", "precompile": "rimraf build", diff --git a/test/deno/cliui-test.ts b/test/deno/cliui-test.ts index ce24068..c72c2c5 100644 --- a/test/deno/cliui-test.ts +++ b/test/deno/cliui-test.ts @@ -36,13 +36,12 @@ Deno.test('evenly divides text across columns if multiple columns are given', () }) // it should wrap each column appropriately. - // TODO: we should flesh out the Deno and ESM implementation - // such that it spreads words out over multiple columns appropriately: const expected = [ - 'i am a string ti am a seconi am a third', - 'hat should be wd string tha string that', - 'rapped t should be should be w', - ' wrapped rapped' + 'i am a string i am a i am a third', + 'that should be second string that', + 'wrapped string that should be', + ' should be wrapped', + ' wrapped', ] ui.toString().split('\n').forEach((line: string, i: number) => { diff --git a/test/esm/cliui-test.mjs b/test/esm/cliui-test.mjs index f57d77d..93bbff4 100644 --- a/test/esm/cliui-test.mjs +++ b/test/esm/cliui-test.mjs @@ -35,13 +35,14 @@ describe('ESM', () => { // TODO: we should flesh out the Deno and ESM implementation // such that it spreads words out over multiple columns appropriately: const expected = [ - 'i am a string ti am a seconi am a third', - 'hat should be wd string tha string that', - 'rapped t should be should be w', - ' wrapped rapped' + 'i am a string i am a i am a third', + 'that should be second string that', + 'wrapped string that should be', + ' should be wrapped', + ' wrapped', ] ui.toString().split('\n').forEach((line, i) => { strictEqual(line, expected[i]) }) }) -}) \ No newline at end of file +}) From 9f97090165675fdda63a79c29bc36bb1033506b0 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 1 May 2023 13:45:12 -0700 Subject: [PATCH 6/6] chore: fix trailing comma causing lint failure --- test/deno/cliui-test.ts | 2 +- test/esm/cliui-test.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/deno/cliui-test.ts b/test/deno/cliui-test.ts index c72c2c5..cae566f 100644 --- a/test/deno/cliui-test.ts +++ b/test/deno/cliui-test.ts @@ -41,7 +41,7 @@ Deno.test('evenly divides text across columns if multiple columns are given', () 'that should be second string that', 'wrapped string that should be', ' should be wrapped', - ' wrapped', + ' wrapped' ] ui.toString().split('\n').forEach((line: string, i: number) => { diff --git a/test/esm/cliui-test.mjs b/test/esm/cliui-test.mjs index 93bbff4..c61715d 100644 --- a/test/esm/cliui-test.mjs +++ b/test/esm/cliui-test.mjs @@ -39,7 +39,7 @@ describe('ESM', () => { 'that should be second string that', 'wrapped string that should be', ' should be wrapped', - ' wrapped', + ' wrapped' ] ui.toString().split('\n').forEach((line, i) => { strictEqual(line, expected[i])