Skip to content

Commit

Permalink
fix: consolidate bugs, docs, repo command logic (#4857)
Browse files Browse the repository at this point in the history
All three of these commands do the same thing: open a manifest and find
a url inside to open it.  The finding of that manifest was not very
consistent across these three commands. Some work with workspaces while
others don't. Some work correctly with `--prefix` while others don't.

This PR consolidates these commands so that they all are consistent in
how they find the manifest being referenced.  The specifics of which url
they open are still left to each command.  The util that only these
three commands were using was consolidated into their base class.
  • Loading branch information
wraithgar authored May 9, 2022
1 parent 86f443e commit 5baa4a7
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 149 deletions.
64 changes: 63 additions & 1 deletion docs/content/commands/npm-bugs.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ description: Report bugs for a package in a web browser
<!-- see lib/commands/bugs.js -->

```bash
npm bugs [<pkgname>]
npm bugs [<pkgname> [<pkgname> ...]]

alias: issues
```
Expand Down Expand Up @@ -58,6 +58,68 @@ The base URL of the npm registry.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
#### `workspace`
* Default:
* Type: String (can be set multiple times)
Enable running a command in the context of the configured workspaces of the
current project while filtering by running only the workspaces defined by
this configuration option.
Valid values for the `workspace` config are either:
* Workspace names
* Path to a workspace directory
* Path to a parent workspace directory (will result in selecting all
workspaces within that folder)
When set for the `npm init` command, this may be set to the folder of a
workspace which does not yet exist, to create the folder and set it up as a
brand new workspace within the project.
This value is not exported to the environment for child processes.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
#### `workspaces`
* Default: null
* Type: null or Boolean
Set to true to run the command in the context of **all** configured
workspaces.
Explicitly setting this to false will cause commands like `install` to
ignore workspaces altogether. When not set explicitly:
- Commands that operate on the `node_modules` tree (install, update, etc.)
will link workspaces into the `node_modules` folder. - Commands that do
other things (test, exec, publish, etc.) will operate on the root project,
_unless_ one or more workspaces are specified in the `workspace` config.
This value is not exported to the environment for child processes.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
#### `include-workspace-root`
* Default: false
* Type: Boolean
Include the workspace root when workspaces are enabled for a command.
When false, specifying individual workspaces via the `workspace` config, or
all workspaces via the `workspaces` flag, will cause npm to operate only on
the specified workspaces, and not on the root project.
This value is not exported to the environment for child processes.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
<!-- AUTOGENERATED CONFIG DESCRIPTIONS END -->
### See Also
Expand Down
10 changes: 10 additions & 0 deletions docs/content/commands/npm-repo.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ Set to `true` to use default system URL opener.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
#### `registry`
* Default: "https://registry.npmjs.org/"
* Type: URL
The base URL of the npm registry.
<!-- automatically generated, do not edit manually -->
<!-- see lib/utils/config/definitions.js -->
#### `workspace`
* Default:
Expand Down
31 changes: 4 additions & 27 deletions lib/commands/bugs.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,10 @@
const pacote = require('pacote')
const log = require('../utils/log-shim')
const openUrl = require('../utils/open-url.js')
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
const BaseCommand = require('../base-command.js')
const PackageUrlCmd = require('../package-url-cmd.js')

class Bugs extends BaseCommand {
class Bugs extends PackageUrlCmd {
static description = 'Report bugs for a package in a web browser'
static name = 'bugs'
static usage = ['[<pkgname>]']
static params = ['browser', 'registry']
static ignoreImplicitWorkspace = true

async exec (args) {
if (!args || !args.length) {
args = ['.']
}

await Promise.all(args.map(pkg => this.getBugs(pkg)))
}

async getBugs (pkg) {
const opts = { ...this.npm.flatOptions, fullMetadata: true }
const mani = await pacote.manifest(pkg, opts)
const url = this.getBugsUrl(mani)
log.silly('bugs', 'url', url)
await openUrl(this.npm, url, `${mani.name} bug list available at the following URL`)
}

getBugsUrl (mani) {
getUrl (spec, mani) {
if (mani.bugs) {
if (typeof mani.bugs === 'string') {
return mani.bugs
Expand All @@ -43,7 +20,7 @@ class Bugs extends BaseCommand {
}

// try to get it from the repo, if possible
const info = hostedFromMani(mani)
const info = this.hostedFromMani(mani)
if (info) {
return info.bugs()
}
Expand Down
45 changes: 5 additions & 40 deletions lib/commands/docs.js
Original file line number Diff line number Diff line change
@@ -1,54 +1,19 @@
const pacote = require('pacote')
const openUrl = require('../utils/open-url.js')
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
const log = require('../utils/log-shim')
const BaseCommand = require('../base-command.js')
class Docs extends BaseCommand {
const PackageUrlCmd = require('../package-url-cmd.js')
class Docs extends PackageUrlCmd {
static description = 'Open documentation for a package in a web browser'
static name = 'docs'
static params = [
'browser',
'registry',
'workspace',
'workspaces',
'include-workspace-root',
]

static usage = ['[<pkgname> [<pkgname> ...]]']
static ignoreImplicitWorkspace = false

async exec (args) {
if (!args || !args.length) {
args = ['.']
}

await Promise.all(args.map(pkg => this.getDocs(pkg)))
}

async execWorkspaces (args, filters) {
await this.setWorkspaces(filters)
return this.exec(this.workspacePaths)
}

async getDocs (pkg) {
const opts = { ...this.npm.flatOptions, fullMetadata: true }
const mani = await pacote.manifest(pkg, opts)
const url = this.getDocsUrl(mani)
log.silly('docs', 'url', url)
await openUrl(this.npm, url, `${mani.name} docs available at the following URL`)
}

getDocsUrl (mani) {
getUrl (spec, mani) {
if (mani.homepage) {
return mani.homepage
}

const info = hostedFromMani(mani)
const info = this.hostedFromMani(mani)
if (info) {
return info.docs()
}

return 'https://www.npmjs.com/package/' + mani.name
return `https://www.npmjs.com/package/${mani.name}`
}
}
module.exports = Docs
45 changes: 7 additions & 38 deletions lib/commands/repo.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,11 @@
const pacote = require('pacote')
const { URL } = require('url')
const log = require('../utils/log-shim')
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
const openUrl = require('../utils/open-url.js')

const BaseCommand = require('../base-command.js')
class Repo extends BaseCommand {
const PackageUrlCmd = require('../package-url-cmd.js')
class Repo extends PackageUrlCmd {
static description = 'Open package repository page in the browser'
static name = 'repo'
static params = ['browser', 'workspace', 'workspaces', 'include-workspace-root']
static usage = ['[<pkgname> [<pkgname> ...]]']
static ignoreImplicitWorkspace = false

async exec (args) {
if (!args || !args.length) {
args = ['.']
}

await Promise.all(args.map(pkg => this.get(pkg)))
}

async execWorkspaces (args, filters) {
await this.setWorkspaces(filters)
return this.exec(this.workspacePaths)
}

async get (pkg) {
// XXX It is very odd that `where` is how pacote knows to look anywhere
// other than the cwd.
const opts = {
...this.npm.flatOptions,
where: this.npm.localPrefix,
fullMetadata: true,
}
const mani = await pacote.manifest(pkg, opts)

getUrl (spec, mani) {
const r = mani.repository
const rurl = !r ? null
: typeof r === 'string' ? r
Expand All @@ -43,22 +14,20 @@ class Repo extends BaseCommand {

if (!rurl) {
throw Object.assign(new Error('no repository'), {
pkgid: pkg,
pkgid: spec,
})
}

const info = hostedFromMani(mani)
const info = this.hostedFromMani(mani)
const url = info ?
info.browse(mani.repository.directory) : unknownHostedUrl(rurl)

if (!url) {
throw Object.assign(new Error('no repository: could not get url'), {
pkgid: pkg,
pkgid: spec,
})
}

log.silly('docs', 'url', url)
await openUrl(this.npm, url, `${mani.name} repo available at the following URL`)
return url
}
}
module.exports = Repo
Expand Down
61 changes: 61 additions & 0 deletions lib/package-url-cmd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Base command for opening urls from a package manifest (bugs, docs, repo)

const pacote = require('pacote')
const hostedGitInfo = require('hosted-git-info')

const openUrl = require('./utils/open-url.js')
const log = require('./utils/log-shim')

const BaseCommand = require('./base-command.js')
class PackageUrlCommand extends BaseCommand {
static ignoreImplicitWorkspace = false
static params = [
'browser',
'registry',
'workspace',
'workspaces',
'include-workspace-root',
]

static usage = ['[<pkgname> [<pkgname> ...]]']

async exec (args) {
if (!args || !args.length) {
args = ['.']
}

for (const arg of args) {
// XXX It is very odd that `where` is how pacote knows to look anywhere
// other than the cwd.
const opts = {
...this.npm.flatOptions,
where: this.npm.localPrefix,
fullMetadata: true,
}
const mani = await pacote.manifest(arg, opts)
const url = this.getUrl(arg, mani)
log.silly(this.name, 'url', url)
await openUrl(this.npm, url, `${mani.name} ${this.name} available at the following URL`)
}
}

async execWorkspaces (args, filters) {
await this.setWorkspaces(filters)
return this.exec(this.workspacePaths)
}

// given a manifest, try to get the hosted git info from it based on
// repository (if a string) or repository.url (if an object) returns null
// if it's not a valid repo, or not a known hosted repo
hostedFromMani (mani) {
const r = mani.repository
const rurl = !r ? null
: typeof r === 'string' ? r
: typeof r === 'object' && typeof r.url === 'string' ? r.url
: null

// hgi returns undefined sometimes, but let's always return null here
return (rurl && hostedGitInfo.fromUrl(rurl.replace(/^git\+/, ''))) || null
}
}
module.exports = PackageUrlCommand
14 changes: 0 additions & 14 deletions lib/utils/hosted-git-info-from-manifest.js

This file was deleted.

6 changes: 4 additions & 2 deletions tap-snapshots/test/lib/load-all-commands.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ exports[`test/lib/load-all-commands.js TAP load each command bugs > must match s
Report bugs for a package in a web browser
Usage:
npm bugs [<pkgname>]
npm bugs [<pkgname> [<pkgname> ...]]
Options:
[--no-browser|--browser <browser>] [--registry <registry>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root]
alias: issues
Expand Down Expand Up @@ -727,7 +729,7 @@ Usage:
npm repo [<pkgname> [<pkgname> ...]]
Options:
[--no-browser|--browser <browser>]
[--no-browser|--browser <browser>] [--registry <registry>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root]
Expand Down
6 changes: 4 additions & 2 deletions tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,12 @@ All commands:
bugs Report bugs for a package in a web browser
Usage:
npm bugs [<pkgname>]
npm bugs [<pkgname> [<pkgname> ...]]
Options:
[--no-browser|--browser <browser>] [--registry <registry>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root]
alias: issues
Expand Down Expand Up @@ -777,7 +779,7 @@ All commands:
npm repo [<pkgname> [<pkgname> ...]]
Options:
[--no-browser|--browser <browser>]
[--no-browser|--browser <browser>] [--registry <registry>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root]
Expand Down
Loading

0 comments on commit 5baa4a7

Please sign in to comment.