From 1e9c31c4e3929483580a0a554d7515095b5418ca Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 4 Nov 2021 07:45:56 -0700 Subject: [PATCH] fix(help|edit): use npm.exec, use file:/// Windows no longer allows `file://`, but instead requires `file:///` for local file urls. Also after the command refactor things shifted and the tests didn't catch that the help pages didn't work at all anymore, and that the post-edit rebuild didn't work. This cleans up those two issues. PR-URL: https://github.com/npm/cli/pull/3994 Credit: @wraithgar Close: #3994 Reviewed-by: @lukekarrys --- lib/base-command.js | 2 +- lib/commands/edit.js | 7 +------ lib/commands/help-search.js | 2 +- lib/commands/help.js | 17 ++++------------- test/lib/commands/edit.js | 9 ++++----- test/lib/commands/help-search.js | 7 +++---- test/lib/commands/help.js | 12 +++++------- 7 files changed, 19 insertions(+), 37 deletions(-) diff --git a/lib/base-command.js b/lib/base-command.js index 011429f6acdb7..9657110f83291 100644 --- a/lib/base-command.js +++ b/lib/base-command.js @@ -1,4 +1,4 @@ -// Base class for npm.commands[cmd] +// Base class for npm commands const usageUtil = require('./utils/usage.js') const ConfigDefinitions = require('./utils/config/definitions.js') const getWorkspaces = require('./workspaces/get-workspaces.js') diff --git a/lib/commands/edit.js b/lib/commands/edit.js index 26fb18b2997c1..db9be4a267bfa 100644 --- a/lib/commands/edit.js +++ b/lib/commands/edit.js @@ -50,12 +50,7 @@ class Edit extends BaseCommand { editor.on('exit', (code) => { if (code) return reject(new Error(`editor process exited with code: ${code}`)) - this.npm.commands.rebuild([dir], (err) => { - if (err) - return reject(err) - - resolve() - }) + this.npm.exec('rebuild', [dir]).catch(reject).then(resolve) }) }) }) diff --git a/lib/commands/help-search.js b/lib/commands/help-search.js index a9acf21c65c62..4ab4a0896af44 100644 --- a/lib/commands/help-search.js +++ b/lib/commands/help-search.js @@ -30,7 +30,7 @@ class HelpSearch extends BaseCommand { if (!args.length) return this.npm.output(this.usage) - const docPath = path.resolve(__dirname, '..', 'docs/content') + const docPath = path.resolve(__dirname, '..', '..', 'docs/content') const files = await glob(`${docPath}/*/*.md`) const data = await this.readFiles(files) const results = await this.searchFiles(args, data, files) diff --git a/lib/commands/help.js b/lib/commands/help.js index 5b6a87bfc5a01..bfc7f8b60e5b3 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -36,7 +36,7 @@ class Help extends BaseCommand { async completion (opts) { if (opts.conf.argv.remain.length > 2) return [] - const g = path.resolve(__dirname, '../man/man[0-9]/*.[0-9]') + const g = path.resolve(__dirname, '../../man/man[0-9]/*.[0-9]') const files = await glob(g) return Object.keys(files.reduce(function (acc, file) { @@ -66,7 +66,7 @@ class Help extends BaseCommand { // support `npm help package.json` section = section.replace('.json', '-json') - const manroot = path.resolve(__dirname, '..', 'man') + const manroot = path.resolve(__dirname, '..', '..', 'man') // find either section.n or npm-section.n const f = `${manroot}/${manSearch}/?(npm-)${section}.[0-9]*` let mans = await glob(f) @@ -90,16 +90,7 @@ class Help extends BaseCommand { } helpSearch (args) { - return new Promise((resolve, reject) => { - this.npm.commands['help-search'](args, (err) => { - // This would only error if args was empty, which it never is - /* istanbul ignore next */ - if (err) - return reject(err) - - resolve() - }) - }) + return this.npm.exec('help-search', args) } async viewMan (man) { @@ -157,7 +148,7 @@ class Help extends BaseCommand { sect = 'using-npm' break } - return 'file://' + path.resolve(__dirname, '..', 'docs', 'output', sect, f + '.html') + return 'file:///' + path.resolve(__dirname, '..', '..', 'docs', 'output', sect, f + '.html') } } module.exports = Help diff --git a/test/lib/commands/edit.js b/test/lib/commands/edit.js index 29d18babfa76b..39e1697b71c1e 100644 --- a/test/lib/commands/edit.js +++ b/test/lib/commands/edit.js @@ -29,11 +29,10 @@ const npm = { get: () => EDITOR, }, dir: resolve(__dirname, '../../../node_modules'), - commands: { - rebuild: (args, cb) => { - rebuildArgs = args - return cb(rebuildFail) - }, + exec: async (cmd, args) => { + rebuildArgs = args + if (rebuildFail) + throw rebuildFail }, } diff --git a/test/lib/commands/help-search.js b/test/lib/commands/help-search.js index 8fc50d7467713..9faa38a32fd02 100644 --- a/test/lib/commands/help-search.js +++ b/test/lib/commands/help-search.js @@ -19,10 +19,9 @@ const npm = mockNpm({ long: false, }, usage: 'npm test usage', - commands: { - help: (args, cb) => { - return cb(npmHelpErr) - }, + exec: async () => { + if (npmHelpErr) + throw npmHelpErr }, output, }) diff --git a/test/lib/commands/help.js b/test/lib/commands/help.js index 3abf46c9a0b6e..9ea2b9d92ae7e 100644 --- a/test/lib/commands/help.js +++ b/test/lib/commands/help.js @@ -20,14 +20,11 @@ const npm = { cooked: [], }, }, - commands: { - 'help-search': (args, cb) => { + exec: async (cmd, args) => { + if (cmd === 'help-search') helpSearchArgs = args - return cb() - }, - help: { - usage: 'npm help ', - }, + else if (cmd === 'help') + return { usage: 'npm help ' } }, deref: (cmd) => {}, output: msg => { @@ -162,6 +159,7 @@ t.test('npm help 1 install', async t => { await help.exec(['1', 'install']) t.match(openUrlArg, /commands(\/|\\)npm-install.html$/, 'attempts to open the correct url') + t.ok(openUrlArg.startsWith('file:///'), 'opens with the correct uri schema') }) t.test('npm help 5 install', async t => {