From db51bf216b4b37d6c6e1d054e64b38dff0856d6d Mon Sep 17 00:00:00 2001 From: Andrea Corbellini Date: Thu, 29 Jun 2023 13:39:56 -0700 Subject: [PATCH 1/3] fix: properly truncate table cells that contain fullwidth characters or ANSI escape sequences --- package.json | 4 +++- src/cli-ux/styled/table.ts | 8 +++++++- test/cli-ux/styled/table.test.ts | 35 +++++++++++++++++++++++++++++++- yarn.lock | 5 +++++ 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 259c2697e..7263ebe05 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "object-treeify": "^1.1.33", "password-prompt": "^1.1.2", "semver": "^7.5.3", + "slice-ansi": "^4.0.0", "string-width": "^4.2.3", "strip-ansi": "^6.0.1", "supports-color": "^8.1.1", @@ -55,6 +56,7 @@ "@types/proxyquire": "^1.3.28", "@types/semver": "^7.5.0", "@types/shelljs": "^0.8.11", + "@types/slice-ansi": "^4.0.0", "@types/strip-ansi": "^5.2.1", "@types/supports-color": "^8.1.1", "@types/wordwrap": "^1.0.1", @@ -114,4 +116,4 @@ "pretest": "yarn build --noEmit && tsc -p test --noEmit --skipLibCheck" }, "types": "lib/index.d.ts" -} \ No newline at end of file +} diff --git a/src/cli-ux/styled/table.ts b/src/cli-ux/styled/table.ts index 962c89caf..04f8b0cf2 100644 --- a/src/cli-ux/styled/table.ts +++ b/src/cli-ux/styled/table.ts @@ -9,6 +9,7 @@ import {stdout} from '../stream' const sw = require('string-width') const {orderBy} = require('natural-orderby') +const sliceAnsi = require('slice-ansi') class Table> { options: table.Options & { printLine(s: any): any } @@ -284,7 +285,12 @@ class Table> { const colorWidth = (d.length - visualWidth) let cell = d.padEnd(width + colorWidth) if ((cell.length - colorWidth) > width || visualWidth === width) { - cell = cell.slice(0, width - 2) + '… ' + // truncate the cell, preserving ANSI escape sequences, and keeping + // into account the width of fullwidth unicode characters + cell = sliceAnsi(cell, 0, width - 2) + '… ' + // pad with spaces; this is necessary in case the original string + // contained fullwidth characters which cannot be split + cell += ' '.repeat(width - sw(cell)) } l += cell diff --git a/test/cli-ux/styled/table.test.ts b/test/cli-ux/styled/table.test.ts index 3383fb5bd..977954dd4 100644 --- a/test/cli-ux/styled/table.test.ts +++ b/test/cli-ux/styled/table.test.ts @@ -1,6 +1,7 @@ import {expect, fancy} from 'fancy-test' import {ux} from '../../../src' +import * as screen from '../../../src/screen' /* eslint-disable camelcase */ const apps = [ @@ -317,6 +318,38 @@ describe('styled/table', () => { 321 supertable-test-2${ws} 123 supertable-test-1${ws}\n`) }) + + const orig = { + stdtermwidth: screen.stdtermwidth, + CLI_UX_SKIP_TTY_CHECK: process.env.CLI_UX_SKIP_TTY_CHECK, + } + + fancy + .do(() => { + Object.assign(screen, {stdtermwidth: 9}) + process.env.CLI_UX_SKIP_TTY_CHECK = 'true' + }) + .finally(() => { + Object.assign(screen, {stdtermwidth: orig.stdtermwidth}) + process.env.CLI_UX_SKIP_TTY_CHECK = orig.CLI_UX_SKIP_TTY_CHECK + }) + .stdout({stripColor: false}) + .end('correctly truncates columns with fullwidth characters or ansi escape sequences', output => { + /* eslint-disable camelcase */ + const app4 = { + build_stack: { + name: 'heroku-16', + }, + id: '456', + name: '\u001B[31m超级表格—测试\u001B[0m', + web_url: 'https://supertable-test-1.herokuapp.com/', + } + /* eslint-enable camelcase */ + + ux.table([...apps, app4 as any], {name: {}}, {'no-header': true}) + expect(output.stdout).to.equal(` super…${ws} + super…${ws} + \u001B[31m超级\u001B[39m…${ws}${ws}\n`) + }) }) }) - diff --git a/yarn.lock b/yarn.lock index 3cdead2f3..a4e45c202 100644 --- a/yarn.lock +++ b/yarn.lock @@ -722,6 +722,11 @@ dependencies: "@sinonjs/fake-timers" "^7.1.0" +"@types/slice-ansi@^4.0.0": + version "4.0.0" + resolved "https://registry.yarnpkg.com/@types/slice-ansi/-/slice-ansi-4.0.0.tgz#eb40dfbe3ac5c1de61f6bcb9ed471f54baa989d6" + integrity sha512-+OpjSaq85gvlZAYINyzKpLeiFkSC4EsC6IIiT6v6TLSU5k5U83fHGj9Lel8oKEXM0HqgrMVCjXPDPVICtxF7EQ== + "@types/strip-ansi@^5.2.1": version "5.2.1" resolved "https://registry.yarnpkg.com/@types/strip-ansi/-/strip-ansi-5.2.1.tgz#acd97f1f091e332bb7ce697c4609eb2370fa2a92" From 766859e2a5af2ec5fe2024084a249b3f7516d509 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 10 Jul 2023 12:44:22 -0600 Subject: [PATCH 2/3] fix: improve flag validation * test: add external NUTs * test: create external-nuts to run correct yarn command * test: use plugin repo URL * test: use oclif/github-workflows repo * test: remove external-nuts file * test: remove preswap commands * test: disable pa NUTs for a change * test: enable pa NUT * test: disable pa? * test: add preswap commands * chore: let me clone these repos please * chore: skip windows for now * chore: fix space, retry? * chore: fix space, retry? * chore: empty to clear GHA cache * chore: try salesforcecli ext Nuts * chore: add compile * chore: empty to clear GHA cache * chore: pass envs * chore: pass envs as env * chore: back to inherit * fix: secrets param * test: pair down NUTs list * fix: improve flag validation to prevent a flag being read as a flag value * fix(deps): bump semver and @types/semver Bumps [semver](https://github.com/npm/node-semver) and [@types/semver](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/semver). These dependencies needed to be updated together. Updates `semver` from 7.5.0 to 7.5.3 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/main/CHANGELOG.md) - [Commits](https://github.com/npm/node-semver/compare/v7.5.0...v7.5.3) Updates `@types/semver` from 7.3.13 to 7.5.0 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/semver) --- updated-dependencies: - dependency-name: semver dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: "@types/semver" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] * chore(release): 2.8.11 [skip ci] * fix: improve flag validation * chore: review * chore: review * fix: enforce -- prefixes --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: svc-cli-bot <113140650+svc-cli-bot@users.noreply.github.com> Co-authored-by: svc-cli-bot * refactor: object return instead of tuple (#727) * fix(deps): bump semver and @types/semver Bumps [semver](https://github.com/npm/node-semver) and [@types/semver](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/semver). These dependencies needed to be updated together. Updates `semver` from 7.5.0 to 7.5.3 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/main/CHANGELOG.md) - [Commits](https://github.com/npm/node-semver/compare/v7.5.0...v7.5.3) Updates `@types/semver` from 7.3.13 to 7.5.0 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/semver) --- updated-dependencies: - dependency-name: semver dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: "@types/semver" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] * chore(release): 2.8.11 [skip ci] * fix: improve flag validation * chore: review * chore: review * fix: enforce -- prefixes * refactor: object return instead of tuple --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: svc-cli-bot <113140650+svc-cli-bot@users.noreply.github.com> Co-authored-by: svc-cli-bot Co-authored-by: Willie Ruemmele --------- Signed-off-by: dependabot[bot] Co-authored-by: Eric Willhoit Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: svc-cli-bot <113140650+svc-cli-bot@users.noreply.github.com> Co-authored-by: svc-cli-bot Co-authored-by: Shane McLaughlin --- .github/workflows/test.yml | 29 +++++++++++++++- package.json | 3 +- src/parser/parse.ts | 25 +++++++++----- test/parser/parse.test.ts | 70 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 68dcef099..16cae5f81 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,7 +22,7 @@ jobs: exclude: - os: windows-latest node_version: lts/* - - os: windows-latest + - os: windows-latest node_version: lts/-1 fail-fast: false runs-on: ${{ matrix.os }} @@ -35,3 +35,30 @@ jobs: - run: yarn install --network-timeout 600000 - run: yarn build - run: yarn test:e2e + nuts: + needs: linux-unit-tests + uses: salesforcecli/github-workflows/.github/workflows/externalNut.yml@main + strategy: + fail-fast: false + matrix: + os: ["ubuntu-latest", "windows-latest"] + externalProjectGitUrl: + - https://github.com/salesforcecli/plugin-auth + - https://github.com/salesforcecli/plugin-data + - https://github.com/salesforcecli/plugin-org + - https://github.com/salesforcecli/plugin-schema + - https://github.com/salesforcecli/plugin-user + with: + packageName: "@oclif/core" + externalProjectGitUrl: ${{ matrix.externalProjectGitUrl }} + command: "yarn test:nuts" + os: ${{ matrix.os }} + useCache: false + preSwapCommands: "npx yarn-deduplicate; yarn install" + preExternalBuildCommands: "shx rm -rf node_modules/@salesforce/sf-plugins-core/node_modules/@oclif/core" + secrets: + TESTKIT_AUTH_URL: ${{ secrets.TESTKIT_AUTH_URL }} + TESTKIT_HUB_USERNAME: ${{ secrets.TESTKIT_HUB_USERNAME }} + TESTKIT_JWT_CLIENT_ID: ${{ secrets.TESTKIT_JWT_CLIENT_ID }} + TESTKIT_JWT_KEY: ${{ secrets.TESTKIT_JWT_KEY }} + TESTKIT_HUB_INSTANCE: ${{ secrets.TESTKIT_HUB_INSTANCE }} diff --git a/package.json b/package.json index 259c2697e..7085424c9 100644 --- a/package.json +++ b/package.json @@ -108,10 +108,11 @@ "commitlint": "commitlint", "lint": "eslint . --ext .ts --config .eslintrc", "posttest": "yarn lint", + "compile": "tsc", "prepack": "yarn run build", "test": "mocha --forbid-only \"test/**/*.test.ts\"", "test:e2e": "mocha --forbid-only \"test/**/*.e2e.ts\" --timeout 1200000", "pretest": "yarn build --noEmit && tsc -p test --noEmit --skipLibCheck" }, "types": "lib/index.d.ts" -} \ No newline at end of file +} diff --git a/src/parser/parse.ts b/src/parser/parse.ts index ba6ceab35..383fc7459 100644 --- a/src/parser/parse.ts +++ b/src/parser/parse.ts @@ -86,7 +86,7 @@ export class Parser> { this._debugInput() - const findLongFlag = (arg: string) => { + const findLongFlag = (arg: string):string | undefined => { const name = arg.slice(2) if (this.input.flags[name]) { return name @@ -102,17 +102,23 @@ export class Parser { + const findShortFlag = ([_, char]: string):string | undefined => { if (this.flagAliases[char]) { return this.flagAliases[char].name } - return Object.keys(this.input.flags).find(k => this.input.flags[k].char === char) + return Object.keys(this.input.flags).find(k => (this.input.flags[k].char === char && char !== undefined && this.input.flags[k].char !== undefined)) + } + + const findFlag = (arg: string): { name?: string, isLong: boolean } => { + const isLong = arg.startsWith('--') + const short = isLong ? false : arg.startsWith('-') + const name = isLong ? findLongFlag(arg) : (short ? findShortFlag(arg) : undefined) + return {name, isLong} } const parseFlag = (arg: string): boolean => { - const long = arg.startsWith('--') - const name = long ? findLongFlag(arg) : findShortFlag(arg) + const {name, isLong} = findFlag(arg) if (!name) { const i = arg.indexOf('=') if (i !== -1) { @@ -133,16 +139,17 @@ export class Parser 2) { + if (!isLong && arg.length > 2) { this.argv.unshift(`-${arg.slice(2)}`) } } diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts index aa9246de7..62c8eaacf 100644 --- a/test/parser/parse.test.ts +++ b/test/parser/parse.test.ts @@ -84,6 +84,76 @@ describe('parse', () => { expect(Boolean(out.flags.myflag2)).to.equal(true) }) + it('doesn\'t throw when 2nd char in value matches a flag char', async () => { + const out = await parse(['--myflag', 'Ishikawa', '-s', 'value'], { + flags: {myflag: Flags.string(), second: Flags.string({char: 's'})}, + }) + expect(out.flags.myflag).to.equal('Ishikawa') + expect(out.flags.second).to.equal('value') + }) + + it('doesn\'t throw when an unprefixed flag value contains a flag name', async () => { + const out = await parse(['--myflag', 'a-second-place-finish', '-s', 'value'], { + flags: {myflag: Flags.string(), second: Flags.string({char: 's'})}, + }) + expect(out.flags.myflag).to.equal('a-second-place-finish') + expect(out.flags.second).to.equal('value') + }) + + it('throws error when no value provided to required flag', async () => { + try { + await parse(['--myflag', '--second', 'value'], { + flags: {myflag: Flags.string({required: true}), second: Flags.string()}, + }) + assert.fail('should have thrown') + } catch (error) { + expect((error as CLIError).message).to.include('Flag --myflag expects a value') + } + }) + + it('throws error when no value provided to required flag before a short char flag', async () => { + try { + await parse(['--myflag', '-s', 'value'], { + flags: {myflag: Flags.string({required: true}), second: Flags.string({char: 's'})}, + }) + assert.fail('should have thrown') + } catch (error) { + expect((error as CLIError).message).to.include('Flag --myflag expects a value') + } + }) + + it('doesn\'t throw when boolean flag passed', async () => { + const out = await parse(['--myflag', '--second', 'value'], { + flags: {myflag: Flags.boolean(), second: Flags.string()}, + }) + expect(out.flags.myflag).to.be.true + expect(out.flags.second).to.equal('value') + }) + + it('doesn\'t throw when negative number passed', async () => { + const out = await parse(['--myflag', '-s', '-9'], { + flags: {myflag: Flags.boolean(), second: Flags.integer({char: 's'})}, + }) + expect(out.flags.myflag).to.be.true + expect(out.flags.second).to.equal(-9) + }) + + it('doesn\'t throw when boolean short char is passed', async () => { + const out = await parse(['--myflag', '-s', 'value'], { + flags: {myflag: Flags.boolean(), second: Flags.string({char: 's'})}, + }) + expect(out.flags.myflag).to.be.true + expect(out.flags.second).to.equal('value') + }) + + it('doesn\'t throw when short char is passed as a string value', async () => { + const out = await parse(['--myflag', '\'-s\'', '-s', 'value'], { + flags: {myflag: Flags.string(), second: Flags.string({char: 's'})}, + }) + expect(out.flags.myflag).to.equal('\'-s\'') + expect(out.flags.second).to.equal('value') + }) + it('parses short flags', async () => { const out = await parse(['-mf'], { flags: { From 544f115782fcf98ba08681926e0948126d346aad Mon Sep 17 00:00:00 2001 From: svc-cli-bot Date: Mon, 10 Jul 2023 22:39:08 +0000 Subject: [PATCH 3/3] chore(release): 2.8.12 [skip ci] --- CHANGELOG.md | 9 +++++++++ package.json | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0875c78cb..d202152d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## [2.8.12](https://github.com/oclif/core/compare/2.8.11...2.8.12) (2023-07-10) + + +### Bug Fixes + +* properly truncate table cells that contain fullwidth characters or ANSI escape sequences ([db51bf2](https://github.com/oclif/core/commit/db51bf216b4b37d6c6e1d054e64b38dff0856d6d)) + + + ## [2.8.11](https://github.com/oclif/core/compare/2.8.10...2.8.11) (2023-07-01) diff --git a/package.json b/package.json index 426188837..1b72350f3 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@oclif/core", "description": "base library for oclif CLIs", - "version": "2.8.11", + "version": "2.8.12", "author": "Salesforce", "bugs": "https://github.com/oclif/core/issues", "dependencies": { @@ -117,4 +117,4 @@ "pretest": "yarn build --noEmit && tsc -p test --noEmit --skipLibCheck" }, "types": "lib/index.d.ts" -} +} \ No newline at end of file