Skip to content

Commit

Permalink
fix: handle strings the same in cjs, esm, and deno
Browse files Browse the repository at this point in the history
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: yargs#138
  • Loading branch information
isaacs committed May 1, 2023
1 parent 3ed58ce commit 7a37884
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 48 deletions.
11 changes: 6 additions & 5 deletions 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
})
Expand Down
11 changes: 6 additions & 5 deletions 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
})
Expand Down
6 changes: 3 additions & 3 deletions 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,
Expand Down
19 changes: 18 additions & 1 deletion lib/index.ts
Expand Up @@ -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 = []
}

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

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

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand All @@ -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
}
Expand All @@ -383,7 +398,9 @@ let mixin: Mixin
export function cliui (opts: Partial<UIOptions>, _mixin: Mixin) {
mixin = _mixin
return new UI({
/* c8 ignore start */
width: opts?.width || getWindowWidth(),
wrap: opts?.wrap
/* c8 ignore stop */
})
}
30 changes: 0 additions & 30 deletions lib/string-utils.ts

This file was deleted.

9 changes: 6 additions & 3 deletions package.json
Expand Up @@ -49,9 +49,12 @@
"author": "Ben Coe <ben@npmjs.com>",
"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",
Expand Down
34 changes: 34 additions & 0 deletions 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 <key-id>] [-f] [-m <msg> | -F <file>] [-e]
<tagname> [<commit> | <object>]
or: git tag -d <tagname>...
or: git tag [-n[<num>]] -l [--contains <commit>] [--no-contains <commit>]
[--points-at <object>] [--column[=<options>] | --no-column]
[--create-reflog] [--sort=<key>] [--format=<format>]
[--merged <commit>] [--no-merged <commit>] [<pattern>...]
or: git tag -v [--format=<format>] <tagname>...`


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())
})
})
})
2 changes: 1 addition & 1 deletion test/cliui.cjs
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 7a37884

Please sign in to comment.