Skip to content

Commit

Permalink
fix(help|edit): use npm.exec, use file:///
Browse files Browse the repository at this point in the history
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: #3994
Credit: @wraithgar
Close: #3994
Reviewed-by: @lukekarrys
  • Loading branch information
wraithgar committed Nov 4, 2021
1 parent cad9bc7 commit 1e9c31c
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 37 deletions.
2 changes: 1 addition & 1 deletion 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')
Expand Down
7 changes: 1 addition & 6 deletions lib/commands/edit.js
Expand Up @@ -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)
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/help-search.js
Expand Up @@ -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)
Expand Down
17 changes: 4 additions & 13 deletions lib/commands/help.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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
9 changes: 4 additions & 5 deletions test/lib/commands/edit.js
Expand Up @@ -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
},
}

Expand Down
7 changes: 3 additions & 4 deletions test/lib/commands/help-search.js
Expand Up @@ -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,
})
Expand Down
12 changes: 5 additions & 7 deletions test/lib/commands/help.js
Expand Up @@ -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 <term>',
},
else if (cmd === 'help')
return { usage: 'npm help <term>' }
},
deref: (cmd) => {},
output: msg => {
Expand Down Expand Up @@ -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 => {
Expand Down

0 comments on commit 1e9c31c

Please sign in to comment.