diff --git a/lib/commands/help.js b/lib/commands/help.js index 40f5ad9b30092..266b7e40a53f7 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -11,6 +11,8 @@ const BaseCommand = require('../base-command.js') // We don't currently compress our man pages but if we ever did this would // seemlessly continue supporting it const manNumberRegex = /\.(\d+)(\.[^/\\]*)?$/ +// Searches for the "npm-" prefix in page names, to prefer those. +const manNpmPrefixRegex = /\/npm-/ class Help extends BaseCommand { static description = 'Get help on npm' @@ -61,13 +63,27 @@ class Help extends BaseCommand { const f = `${manroot}/${manSearch}/?(npm-)${section}.[0-9]*` let mans = await glob(f) mans = mans.sort((a, b) => { - // Because of the glob we know the manNumberRegex will pass - const aManNumber = a.match(manNumberRegex)[1] - const bManNumber = b.match(manNumberRegex)[1] + // Prefer the page with an npm prefix, if there's only one. + const aHasPrefix = a.match(manNpmPrefixRegex) + const bHasPrefix = b.match(manNpmPrefixRegex) + if (!aHasPrefix !== !bHasPrefix) { + return aHasPrefix ? -1 : 1 + } - // man number sort first so that 1 aka commands are preferred - if (aManNumber !== bManNumber) { - return aManNumber - bManNumber + // 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) { + 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] + } + } else if (bManNumberMatch) { + return 1 } return localeCompare(a, b) diff --git a/test/lib/commands/help.js b/test/lib/commands/help.js index b76234d996627..f84f94ad2f8d9 100644 --- a/test/lib/commands/help.js +++ b/test/lib/commands/help.js @@ -310,3 +310,41 @@ t.test('npm help with complex installation path finds proper help file', async t t.match(openUrlArg, /commands(\/|\\)npm-install.html$/, 'attempts to open the correct url') }) + +t.test('npm help - prefers npm help pages', async t => { + // Unusual ordering is to get full test coverage of all branches inside the + // sort function. + globResult = [ + '/root/man/man6/npm-install.6', + '/root/man/man1/install.1', + '/root/man/man5/npm-install.5', + ] + t.teardown(() => { + globResult = globDefaults + spawnBin = null + spawnArgs = null + }) + await help.exec(['install']) + + t.equal(spawnBin, 'man', 'calls man by default') + t.strictSame(spawnArgs, ['/root/man/man5/npm-install.5'], 'passes the correct arguments') +}) + +t.test('npm help - works in the presence of strange man pages', async t => { + // Unusual ordering is to get full test coverage of all branches inside the + // sort function. + globResult = [ + '/root/man/man6/config.6strange', + '/root/man/man1/config.1', + '/root/man/man5/config.5ssl', + ] + t.teardown(() => { + globResult = globDefaults + spawnBin = null + spawnArgs = null + }) + await help.exec(['config']) + + t.equal(spawnBin, 'man', 'calls man by default') + t.strictSame(spawnArgs, ['/root/man/man1/config.1'], 'passes the correct arguments') +})