From 6a4bcbaaf12c15041c73914fb3a24389a62f7436 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 21 Mar 2023 13:27:32 -0700 Subject: [PATCH] fix: clean up man sorting glob doesn't sort and returns things in the os specific representation. Also a lot of the edge cases here covered non-reality. Our current man setup already has all of the `npm-` prefixed things in `man1`. --- lib/commands/help.js | 30 +++---------------- test/lib/commands/help.js | 27 +++++++++-------- .../test/type-description.js.test.cjs | 5 ++++ workspaces/config/test/fixtures/types.js | 1 + 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/lib/commands/help.js b/lib/commands/help.js index e2d29bf7b16f3..d89f3f6436348 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -11,8 +11,6 @@ const BaseCommand = require('../base-command.js') // We don't currently compress our man pages but if we ever did this would // seamlessly continue supporting it const manNumberRegex = /\.(\d+)(\.[^/\\]*)?$/ -// Searches for the "npm-" prefix in page names, to prefer those. -const manNpmPrefixRegex = /\/npm-/ // hardcoded names for mansections // XXX: these are used in the docs workspace and should be exported // from npm so section names can changed more easily @@ -66,33 +64,13 @@ class Help extends BaseCommand { const f = globify(path.resolve(this.npm.npmRoot, `man/${manSearch}/?(npm-)${arg}.[0-9]*`)) const [man] = await glob(f).then(r => r.sort((a, b) => { - // Prefer the page with an npm prefix, if there's only one. - const aHasPrefix = manNpmPrefixRegex.test(a) - const bHasPrefix = manNpmPrefixRegex.test(b) - if (aHasPrefix !== bHasPrefix) { - /* istanbul ignore next */ - return aHasPrefix ? -1 : 1 - } - // Because the glob is (subtly) different from manNumberRegex, // we can't rely on it passing. - const aManNumberMatch = a.match(manNumberRegex) - const bManNumberMatch = b.match(manNumberRegex) - if (aManNumberMatch) { - /* istanbul ignore next */ - if (!bManNumberMatch) { - return -1 - } - // man number sort first so that 1 aka commands are preferred - if (aManNumberMatch[1] !== bManNumberMatch[1]) { - return aManNumberMatch[1] - bManNumberMatch[1] - } - /* istanbul ignore next */ - } else if (bManNumberMatch) { - /* istanbul ignore next */ - return 1 + const aManNumberMatch = a.match(manNumberRegex)?.[1] || 999 + const bManNumberMatch = b.match(manNumberRegex)?.[1] || 999 + if (aManNumberMatch !== bManNumberMatch) { + return aManNumberMatch - bManNumberMatch } - return localeCompare(a, b) })) diff --git a/test/lib/commands/help.js b/test/lib/commands/help.js index d4e7a81f84a4c..e38f1bbce24d4 100644 --- a/test/lib/commands/help.js +++ b/test/lib/commands/help.js @@ -29,8 +29,8 @@ const genManPages = (obj) => { const mockHelp = async (t, { man = { - 1: ['whoami', 'install', 'star', 'unstar', 'uninstall', 'unpublish'].map(p => `npm-${p}`), 5: ['npmrc', 'install', 'package-json'], + 1: ['whoami', 'install', 'star', 'unstar', 'uninstall', 'unpublish'].map(p => `npm-${p}`), 7: ['disputes', 'config'], }, browser = false, @@ -113,7 +113,7 @@ t.test('npm help whoami', async t => { const [spawnBin, spawnArgs] = getArgs() t.equal(spawnBin, 'man', 'calls man by default') t.equal(spawnArgs.length, 1) - t.match(spawnArgs[0], /\/man\/man1\/npm-whoami\.1$/) + t.match(spawnArgs[0], /npm-whoami\.1$/) }) t.test('npm help 1 install', async t => { @@ -155,7 +155,7 @@ t.test('npm help package.json redirects to package-json', async t => { const [spawnBin, spawnArgs] = getArgs() t.equal(spawnBin, 'man', 'calls man by default') t.equal(spawnArgs.length, 1) - t.match(spawnArgs[0], /\/man\/man5\/package-json\.5$/) + t.match(spawnArgs[0], /package-json\.5$/) }) t.test('npm help ?(un)star', async t => { @@ -168,7 +168,7 @@ t.test('npm help ?(un)star', async t => { t.equal(spawnBin, 'emacsclient', 'maps woman to emacs correctly') t.equal(spawnArgs.length, 2) t.match(spawnArgs[1], /^\(woman-find-file '/) - t.match(spawnArgs[1], /\/man\/man1\/npm-star.1'\)$/) + t.match(spawnArgs[1], /npm-star.1'\)$/) }) t.test('npm help un*', async t => { @@ -179,15 +179,16 @@ t.test('npm help un*', async t => { const [spawnBin, spawnArgs] = getArgs() t.equal(spawnBin, 'man', 'calls man by default') t.equal(spawnArgs.length, 1) - t.match(spawnArgs[0], /\/man\/man1\/npm-uninstall\.1$/) + t.match(spawnArgs[0], /npm-uninstall\.1$/) }) -t.test('npm help - prefers npm help pages', async t => { +t.test('npm help - prefers lowest numbered npm prefixed help pages', async t => { const { getArgs } = await mockHelp(t, { man: { 6: ['npm-install'], - 1: ['install'], - 5: ['install', 'npm-install'], + 1: ['npm-install'], + 5: ['install'], + 7: ['npm-install'], }, exec: ['install'], }) @@ -195,15 +196,15 @@ t.test('npm help - prefers npm help pages', async t => { const [spawnBin, spawnArgs] = getArgs() t.equal(spawnBin, 'man', 'calls man by default') t.equal(spawnArgs.length, 1) - t.match(spawnArgs[0], /\/man\/man5\/npm-install\.5$/) + t.match(spawnArgs[0], /npm-install\.1$/) }) t.test('npm help - works in the presence of strange man pages', async t => { const { getArgs } = await mockHelp(t, { man: { - '6strange': ['config'], - 1: ['config'], - '5ssl': ['config'], + '1strange': ['config'], + 5: ['config'], + '6ssl': ['config'], }, exec: ['config'], }) @@ -211,7 +212,7 @@ t.test('npm help - works in the presence of strange man pages', async t => { const [spawnBin, spawnArgs] = getArgs() t.equal(spawnBin, 'man', 'calls man by default') t.equal(spawnArgs.length, 1) - t.match(spawnArgs[0], /\/man\/man1\/config\.1$/) + t.match(spawnArgs[0], /config\.5$/) }) t.test('rejects with code', async t => { diff --git a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs index 9d80f7e09c315..367731d1d4702 100644 --- a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs +++ b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs @@ -10,6 +10,11 @@ Object { "_exit": Array [ "boolean value (true or false)", ], + "_single": Array [ + Object { + "exotic": "not part of normal typedefs", + }, + ], "access": Array [ null, "restricted", diff --git a/workspaces/config/test/fixtures/types.js b/workspaces/config/test/fixtures/types.js index 0f8cedfd6e6ac..924433e3ec67c 100644 --- a/workspaces/config/test/fixtures/types.js +++ b/workspaces/config/test/fixtures/types.js @@ -148,4 +148,5 @@ module.exports = { versions: Boolean, viewer: String, _exit: Boolean, + _single: { exotic: 'not part of normal typedefs' }, }