From 28a3f0f6addd4dac12b3f566dec8bf81db14184f Mon Sep 17 00:00:00 2001 From: 2colours Date: Sun, 1 Sep 2024 01:38:28 +0200 Subject: [PATCH 1/3] Changing own request.js to async interface This is the version that passes all the existing tests. The HTTP requests used to be handled by a callback that had three arguments: one for the errors if there are any, one for the response and one for the payload within the response. Now, the error has been turned into rejected promises (exceptions when awaited) and the payload has simply been dropped. It's extracted from the response via the .body attribute. --- src/develop.js | 42 +++++------- src/featured.js | 24 +++---- src/install.js | 34 ++++------ src/publish.js | 91 +++++++++----------------- src/request.js | 145 +++++++++++++++++------------------------ src/search.js | 31 ++++----- src/star.js | 36 ++++------ src/stars.js | 30 ++++----- src/theme-converter.js | 22 ++----- src/unpublish.js | 26 +++----- src/unstar.js | 26 +++----- src/upgrade.js | 60 ++++++++--------- src/view.js | 32 ++++----- 13 files changed, 235 insertions(+), 364 deletions(-) diff --git a/src/develop.js b/src/develop.js index 643db746..f80664c8 100644 --- a/src/develop.js +++ b/src/develop.js @@ -43,31 +43,25 @@ cmd-shift-o to run the package out of the newly cloned repository.\ return options.alias('h', 'help').describe('help', 'Print this usage message'); } - getRepositoryUrl(packageName) { - return new Promise((resolve, reject) => { - const requestSettings = { - url: `${config.getAtomPackagesUrl()}/${packageName}`, - json: true - }; - return request.get(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(`Request for package information failed: ${error.message}`); - } - - if (response.statusCode === 200) { - const repositoryUrl = body.repository.url; - if (repositoryUrl) { - return void resolve(repositoryUrl); - } - - return void reject(`No repository URL found for package: ${packageName}`); - } + async getRepositoryUrl(packageName) { + const requestSettings = { + url: `${config.getAtomPackagesUrl()}/${packageName}`, + json: true + }; + const response = await request.get(requestSettings).catch(error => Promise.reject(`Request for package information failed: ${error.message}`)); + const body = response.body ?? {}; + + if (response.statusCode === 200) { + const repositoryUrl = body.repository.url; + if (repositoryUrl) { + return repositoryUrl; + } + + throw `No repository URL found for package: ${packageName}`; + } - const message = request.getErrorMessage(body, error); - return void reject(`Request for package information failed: ${message}`); - }); - }); + const message = request.getErrorMessage(body, error); + throw `Request for package information failed: ${message}`; } async cloneRepository(repoUrl, packageDirectory, options) { diff --git a/src/featured.js b/src/featured.js index 27c1a578..5aeaf88b 100644 --- a/src/featured.js +++ b/src/featured.js @@ -28,7 +28,7 @@ List the Pulsar packages and themes that are currently featured.\ return options.boolean('json').describe('json', 'Output featured packages as JSON array'); } - getFeaturedPackagesByType(atomVersion, packageType) { + async getFeaturedPackagesByType(atomVersion, packageType) { const requestSettings = { url: `${config.getAtomApiUrl()}/${packageType}/featured`, @@ -36,21 +36,17 @@ List the Pulsar packages and themes that are currently featured.\ }; if (atomVersion) { requestSettings.qs = {engine: atomVersion}; } - return new Promise((resolve, reject) => void request.get(requestSettings, function(error, response, body) { - body ??= []; - if (error != null) { - return void reject(error); - } - if (response.statusCode === 200) { - let packages = body.filter(pack => (pack != null ? pack.releases : undefined) != null); + const response = await request.get(requestSettings); + const body = response.body ?? []; + if (response.statusCode === 200) { + let packages = body.filter(pack => pack?.releases != null); packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); packages = _.sortBy(packages, 'name'); - return void resolve(packages); - } - - const message = request.getErrorMessage(body, error); - reject(`Requesting packages failed: ${message}`); - })); + return packages; + } + + const message = request.getErrorMessage(body, error); + throw `Requesting packages failed: ${message}`; } async getAllFeaturedPackages(atomVersion) { diff --git a/src/install.js b/src/install.js index 46a624be..879eb0a0 100644 --- a/src/install.js +++ b/src/install.js @@ -209,32 +209,26 @@ Run ppm -v after installing Git to see what version has been detected.\ // packageName - The string name of the package to request. // // return value - A Promise that rejects with an appropriate error or resolves to the response body - requestPackage(packageName) { + async requestPackage(packageName) { const requestSettings = { url: `${config.getAtomPackagesUrl()}/${packageName}`, json: true, retries: 4 }; - return new Promise((resolve, reject) => { - request.get(requestSettings, (error, response, body) => { - let message; - body ??= {}; - if (error != null) { - message = `Request for package information failed: ${error.message}`; - if (error.status) { message += ` (${error.status})`; } - return void reject(message); - } - if (response.statusCode !== 200) { - message = request.getErrorMessage(body, error); - return void reject(`Request for package information failed: ${message}`); - } - if (!body.releases.latest) { - return void reject(`No releases available for ${packageName}`); - } - - resolve(body); - }); + const response = await request.get(requestSettings).catch(error => { + const message = request.getErrorMessage(body, error); + throw `Request for package information failed: ${message}`; }); + const body = response.body ?? {}; + if (response.statusCode !== 200) { + const message = request.getErrorMessage(body, null); + throw `Request for package information failed: ${message}`; + } + if (!body.releases.latest) { + throw `No releases available for ${packageName}`; + } + + return body; } // Is the package at the specified version already installed? diff --git a/src/publish.js b/src/publish.js index 3d2776dd..8096c67c 100644 --- a/src/publish.js +++ b/src/publish.js @@ -105,35 +105,24 @@ have published it.\ // return value - A Promise that resolves (without a value) when either the // number of max retries have been reached or the tag could // actually be retrieved. - waitForTagToBeAvailable(pack, tag) { - let retryCount = 5; + async waitForTagToBeAvailable(pack, tag) { + const retryCount = 5; const interval = 1000; const requestSettings = { url: `https://api.github.com/repos/${Packages.getRepository(pack)}/tags`, json: true }; - return new Promise((resolve, _reject) => { - const requestTags = () => { - request.get( - requestSettings, - (_error, response, tags) => { - tags ??= []; - if (response?.statusCode === 200) { - if (tags.some(t => t.name === tag)) { - resolve(); - return; - } - } - if (--retryCount <= 0) { - return void resolve(); - } - setTimeout(requestTags, interval); - } - ); - }; - requestTags(); - }); + for (let i = 0; i < retryCount; i++) { + const response = await request.get(requestSettings).catch(); + const tags = response.body ?? []; + if (response?.statusCode === 200) { + if (tags.some(t => t.name === tag)) { + return; + } + } + await new Promise(resolve => setTimeout(resolve, interval)); //not strictly necessary on the last iteration + } } // Does the given package already exist in the registry? @@ -151,15 +140,9 @@ have published it.\ authorization: token } }; - return new Promise((resolve, reject) => { - request.get(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(error); - } - resolve(response.statusCode === 200); - }); - }); + const response = await request.get(requestSettings); + const body = response.body ?? {}; + return response.statusCode === 200; } // Register the current repository with the package registry. @@ -197,22 +180,16 @@ have published it.\ authorization: token } }; - return new Promise((resolve, reject) => { - request.post(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(error); - } - if (response.statusCode !== 201) { - const message = request.getErrorMessage(body, error); - this.logFailure(); - return void reject(`Registering package in ${repository} repository failed: ${message}`); - } + const response = await request.post(requestSettings); + const body = response.body ?? {}; + if (response.statusCode !== 201) { + const message = request.getErrorMessage(body, null); + this.logFailure(); + throw `Registering package in ${repository} repository failed: ${message}`; //again, why the double logging? + } - this.logSuccess(); - return resolve(true); - }); - }); + this.logSuccess(); + return true; } catch (error) { this.logFailure(); throw error; @@ -238,20 +215,12 @@ have published it.\ authorization: token } }; - return new Promise((resolve, reject) => { - request.post(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(error); - } - if (response.statusCode !== 201) { - const message = request.getErrorMessage(body, error); - return void reject(`Creating new version failed: ${message}`); - } - - resolve(); - }); - }); + const response = await request.post(requestSettings); + const body = response.body ?? {}; + if (response.statusCode !== 201) { + const message = request.getErrorMessage(body, null); + throw `Creating new version failed: ${message}`; + } } // Publish the version of the package associated with the given tag. diff --git a/src/request.js b/src/request.js index c4066236..c9f1dd88 100644 --- a/src/request.js +++ b/src/request.js @@ -36,96 +36,69 @@ async function configureRequest(requestOptions){ } module.exports = { - get(opts, callback) { - configureRequest(opts).then(async () => { - const retryCount = opts.retries ?? 0; - - if (typeof opts.strictSSL === "boolean") { - try { - const res = await superagent - .get(opts.url) - .proxy(opts.proxy) - .set(opts.headers) - .query(opts.qs) - .retry(retryCount) - .disableTLSCerts() - .ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - } - - try { - const res = await superagent - .get(opts.url) - .proxy(opts.proxy) - .set(opts.headers) - .query(opts.qs) - .retry(retryCount) - .ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - }); + async get(opts) { + await configureRequest(opts); + const retryCount = opts.retries ?? 0; + + if (typeof opts.strictSSL === "boolean") { + const res = await superagent + .get(opts.url) + .proxy(opts.proxy) + .set(opts.headers) + .query(opts.qs) + .retry(retryCount) + .disableTLSCerts() + .ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; + } + + const res = await superagent + .get(opts.url) + .proxy(opts.proxy) + .set(opts.headers) + .query(opts.qs) + .retry(retryCount) + .ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; }, - del(opts, callback) { - configureRequest(opts).then(async () => { - if (typeof opts.strictSSL === "boolean") { - try { - const res = await superagent - .delete(opts.url) - .proxy(opts.proxy) - .set(opts.headers) - .query(opts.qs) - .disableTLSCerts() - .ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - } - - try { - const res = await superagent - .delete(opts.url) - .proxy(opts.proxy) - .set(opts.headers) - .query(opts.qs) - .ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - }); + async del(opts) { + await configureRequest(opts); + if (typeof opts.strictSSL === "boolean") { + const res = await superagent + .delete(opts.url) + .proxy(opts.proxy) + .set(opts.headers) + .query(opts.qs) + .disableTLSCerts() + .ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; + } + + const res = await superagent + .delete(opts.url) + .proxy(opts.proxy) + .set(opts.headers) + .query(opts.qs) + .ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; }, - post(opts, callback) { - configureRequest(opts).then(async () => { - if (typeof opts.strictSSL === "boolean") { - try { - const res = await superagent - .post(opts.url) - .proxy(opts.proxy) - .set(opts.headers) - .query(opts.qs) - .disableTLSCerts() - .ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - } - - try { - const res = await superagent.post(opts.url).proxy(opts.proxy).set(opts.headers).query(opts.qs).ok((res) => OK_STATUS_CODES.includes(res.status)); - return void callback(null, res, res.body); - } catch (error) { - return void callback(error, null, null); - } - }); + async post(opts) { + await configureRequest(opts); + if (typeof opts.strictSSL === "boolean") { + const res = await superagent + .post(opts.url) + .proxy(opts.proxy) + .set(opts.headers) + .query(opts.qs) + .disableTLSCerts() + .ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; + } + + const res = await superagent.post(opts.url).proxy(opts.proxy).set(opts.headers).query(opts.qs).ok((res) => OK_STATUS_CODES.includes(res.status)); + return res; }, async createReadStream(opts) { diff --git a/src/search.js b/src/search.js index c6fb9e7a..4b34d4cd 100644 --- a/src/search.js +++ b/src/search.js @@ -28,7 +28,7 @@ Search for packages/themes.\ return options.boolean('themes').describe('themes', 'Search only themes').alias('t', 'themes'); } - searchPackages(query, opts) { + async searchPackages(query, opts) { const qs = {q: query}; @@ -44,23 +44,18 @@ Search for packages/themes.\ json: true }; - return new Promise((resolve, reject) => { - request.get(requestSettings, function(error, response, body) { - body ??= {}; - if (error != null) { - return void reject(error); - } - if (response.statusCode === 200) { - let packages = body.filter(pack => (pack.releases != null ? pack.releases.latest : undefined) != null); - packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); - packages = packages.filter(({name, version}) => !isDeprecatedPackage(name, version)); - return void resolve(packages); - } - - const message = request.getErrorMessage(body, error); - reject(`Searching packages failed: ${message}`); - }); - }); + const response = await request.get(requestSettings); + const body = response.body ?? {}; + + if (response.statusCode === 200) { + let packages = body.filter(pack => pack?.releases?.latest != null); + packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); + packages = packages.filter(({name, version}) => !isDeprecatedPackage(name, version)); + return packages; + } + + const message = request.getErrorMessage(body, error); + throw `Searching packages failed: ${message}`; } async run(options) { diff --git a/src/star.js b/src/star.js index 11fd437b..cc018ec3 100644 --- a/src/star.js +++ b/src/star.js @@ -32,7 +32,7 @@ Run \`ppm stars\` to see all your starred packages.\ return options.boolean('installed').describe('installed', 'Star all packages in ~/.pulsar/packages'); } - starPackage(packageName, param) { + async starPackage(packageName, param) { param ??= {}; const {ignoreUnpublishedPackages, token} = param; if (process.platform === 'darwin') { process.stdout.write('\u2B50 '); } @@ -45,27 +45,19 @@ Run \`ppm stars\` to see all your starred packages.\ } }; - return new Promise((resolve, reject) => { - request.post(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - this.logFailure(); - return void reject(error); - } - if ((response.statusCode === 404) && ignoreUnpublishedPackages) { - process.stdout.write('skipped (not published)\n'.yellow); - return void reject(); - } - if (response.statusCode !== 200) { - this.logFailure(); - const message = request.getErrorMessage(body, error); - return void reject(`Starring package failed: ${message}`); - } - - this.logSuccess(); - resolve(); - }); - }); + const response = await request.post(requestSettings).catch(error => { this.logFailure(); throw error; }); + const body = response.body ?? {}; + if ((response.statusCode === 404) && ignoreUnpublishedPackages) { + process.stdout.write('skipped (not published)\n'.yellow); + return Promise.reject(); + } + if (response.statusCode !== 200) { + this.logFailure(); + const message = request.getErrorMessage(body, error); + throw `Starring package failed: ${message}`; + } + + this.logSuccess(); } getInstalledPackageNames() { diff --git a/src/stars.js b/src/stars.js index eb0f7bd6..e6fd3b15 100644 --- a/src/stars.js +++ b/src/stars.js @@ -47,24 +47,18 @@ List or install starred Atom packages and themes.\ return this.requestStarredPackages(requestSettings); } - requestStarredPackages(requestSettings) { - return new Promise((resolve, reject) => { - request.get(requestSettings, (error, response, body) => { - body ??= []; - if (error != null) { - return void reject(error); - } - if (response.statusCode === 200) { - let packages = body.filter(pack => pack?.releases?.latest != null); - packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); - packages = _.sortBy(packages, 'name'); - return void resolve(packages); - } - - const message = request.getErrorMessage(body, error); - reject(`Requesting packages failed: ${message}`); - }); - }); + async requestStarredPackages(requestSettings) { + const response = await request.get(requestSettings); + const body = response.body ?? []; + if (response.statusCode === 200) { + let packages = body.filter(pack => pack?.releases?.latest != null); + packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); + packages = _.sortBy(packages, 'name'); + return packages; + } + + const message = request.getErrorMessage(body, null); + throw `Requesting packages failed: ${message}`; } async installPackages(packages) { diff --git a/src/theme-converter.js b/src/theme-converter.js index 33e508c1..097686b8 100644 --- a/src/theme-converter.js +++ b/src/theme-converter.js @@ -17,21 +17,13 @@ class ThemeConverter { const {protocol} = url.parse(this.sourcePath); if ((protocol === 'http:') || (protocol === 'https:')) { const requestOptions = {url: this.sourcePath}; - return new Promise((resolve, reject) => { - request.get(requestOptions, (error, response, body) => { - if (error != null) { - if (error?.code === 'ENOTFOUND') { - error = `Could not resolve URL: ${this.sourcePath}`; - } - return void reject(error); - } - if (response.statusCode !== 200) { - return void reject(`Request to ${this.sourcePath} failed (${response.headers.status})`); - } - - resolve(body); - }); - }); + const response = await request.get(requestOptions).catch(error => Promise.reject(error?.code === 'ENOTFOUND' ? `Could not resolve URL: ${this.sourcePath}` : error)); + const body = response.body; + if (response.statusCode !== 200) { + throw `Request to ${this.sourcePath} failed (${response.headers.status})`; + } + + return body; } const sourcePath = path.resolve(this.sourcePath); diff --git a/src/unpublish.js b/src/unpublish.js index c5eb73e4..2b3210a5 100644 --- a/src/unpublish.js +++ b/src/unpublish.js @@ -49,23 +49,15 @@ name is specified.\ if (packageVersion) { options.url += `/versions/${packageVersion}`; } - return new Promise((resolve, reject) =>{ - request.del(options, (error, response, body) => { - body ??= {}; - if (error != null) { - this.logFailure(); - return void reject(error); - } - if (response.statusCode !== 204) { - this.logFailure(); - const message = body.message ?? body.error ?? body; - return void reject(`Unpublishing failed: ${message}`); - } - - this.logSuccess(); - resolve(); - }); - }); + const response = await request.del(options).catch(error => { this.logFailure(); throw error; }); //it's not clear why we are calling logFailure in two layers (here and in the usual try-catch around it) + const body = response.body ?? {}; + if (response.statusCode !== 204) { + this.logFailure(); + const message = body.message ?? body.error ?? body; + throw `Unpublishing failed: ${message}`; + } + + this.logSuccess(); } catch (error) { this.logFailure(); throw error; diff --git a/src/unstar.js b/src/unstar.js index 60e75981..cb0abeaf 100644 --- a/src/unstar.js +++ b/src/unstar.js @@ -25,7 +25,7 @@ Run \`ppm stars\` to see all your starred packages.\ return options.alias('h', 'help').describe('help', 'Print this usage message'); } - starPackage(packageName, token) { + async starPackage(packageName, token) { if (process.platform === 'darwin') { process.stdout.write('\uD83D\uDC5F \u2B50 '); } process.stdout.write(`Unstarring ${packageName} `); const requestSettings = { @@ -35,23 +35,15 @@ Run \`ppm stars\` to see all your starred packages.\ authorization: token } }; - return new Promise((resolve, reject) => { - request.del(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - this.logFailure(); - return void reject(error); - } - if (response.statusCode !== 204) { - this.logFailure(); - const message = request.getErrorMessage(body, error); - return void reject(`Unstarring package failed: ${message}`); - } + const response = await request.del(requestSettings).catch(error => { this.logFailure(); throw error; }); + const body = response.body ?? {}; + if (response.statusCode !== 204) { + this.logFailure(); + const message = request.getErrorMessage(body, error); + throw `Unstarring package failed: ${message}`; + } - this.logSuccess(); - resolve(); - }); - }); + this.logSuccess(); } async run(options) { diff --git a/src/upgrade.js b/src/upgrade.js index 2532d6a2..b5c5f442 100644 --- a/src/upgrade.js +++ b/src/upgrade.js @@ -94,49 +94,43 @@ available updates.\ return fs.existsSync(repoGitFolderPath); } - getLatestVersion(pack) { + async getLatestVersion(pack) { const requestSettings = { url: `${config.getAtomPackagesUrl()}/${pack.name}`, json: true }; - return new Promise((resolve, reject) => { - request.get(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(`Request for package information failed: ${error.message}`); - } - if (response.statusCode === 404) { - return void resolve(); - } - if (response.statusCode !== 200) { - const message = body.message ?? body.error ?? body; - return void reject(`Request for package information failed: ${message}`); - } + const response = await request.get(requestSettings).catch(error => Promise.reject(`Request for package information failed: ${error.message}`)); + const body = response.body ?? {}; + if (response.statusCode === 404) { + return; + } + if (response.statusCode !== 200) { + const message = body.message ?? body.error ?? body; + throw `Request for package information failed: ${message}`; + } - const atomVersion = this.installedAtomVersion; - let latestVersion = pack.version; - const object = body?.versions ?? {}; - for (let version in object) { - const metadata = object[version]; - if (!semver.valid(version)) { continue; } - if (!metadata) { continue; } + const atomVersion = this.installedAtomVersion; + let latestVersion = pack.version; + const object = body?.versions ?? {}; + for (let version in object) { + const metadata = object[version]; + if (!semver.valid(version)) { continue; } + if (!metadata) { continue; } - const engine = metadata.engines?.pulsar || metadata.engines?.atom || '*'; - if (!semver.validRange(engine)) { continue; } - if (!semver.satisfies(atomVersion, engine)) { continue; } + const engine = metadata.engines?.pulsar || metadata.engines?.atom || '*'; + if (!semver.validRange(engine)) { continue; } + if (!semver.satisfies(atomVersion, engine)) { continue; } - if (!semver.gt(version, latestVersion)) { continue; } + if (!semver.gt(version, latestVersion)) { continue; } - latestVersion = version; - } + latestVersion = version; + } - if ((latestVersion === pack.version) || !this.hasRepo(pack)) { - return void resolve(); - } + if ((latestVersion === pack.version) || !this.hasRepo(pack)) { + return; + } - resolve(latestVersion); - }); - }); + return latestVersion; } async getLatestSha(pack) { diff --git a/src/view.js b/src/view.js index ec99bb07..35cbb7fc 100644 --- a/src/view.js +++ b/src/view.js @@ -68,30 +68,24 @@ View information about a package/theme.\ } } - getPackage(packageName, options) { + async getPackage(packageName, options) { const requestSettings = { url: `${config.getAtomPackagesUrl()}/${packageName}`, json: true }; - return new Promise((resolve, reject) => { - request.get(requestSettings, (error, response, body) => { - body ??= {}; - if (error != null) { - return void reject(error); - } - if (response.statusCode !== 200) { - const message = body.message ?? body.error ?? body; - return void reject(`Requesting package failed: ${message}`); - } + + const response = await request.get(requestSettings); + const body = response.body ?? {}; + if (response.statusCode !== 200) { + const message = body.message ?? body.error ?? body; + throw `Requesting package failed: ${message}`; + } - this.getLatestCompatibleVersion(body, options).then(version => { - const {name, readme, downloads, stargazers_count} = body; - const metadata = body.versions?.[version] ?? {name}; - const pack = _.extend({}, metadata, {readme, downloads, stargazers_count}); - resolve(pack); - }); - }); - }); + const version = await this.getLatestCompatibleVersion(body, options); + const {name, readme, downloads, stargazers_count} = body; + const metadata = body.versions?.[version] ?? {name}; + const pack = _.extend({}, metadata, {readme, downloads, stargazers_count}); + return pack; } async run(options) { From 358d44e7abfcd4cef995084ea8dd4ea67851742f Mon Sep 17 00:00:00 2001 From: 2colours Date: Sun, 1 Dec 2024 14:05:33 +0100 Subject: [PATCH 2/3] Remove uses of underscore where it's obvious --- src/apm-cli.js | 8 +------- src/ci.js | 7 +++++-- src/command.js | 3 +-- src/config.js | 7 +++++-- src/dedupe.js | 7 +++++-- src/develop.js | 12 ++++++------ src/disable.js | 10 +++++----- src/enable.js | 8 ++++---- src/featured.js | 10 ++++++++-- src/install.js | 26 ++++++++++++++++++++------ src/list.js | 1 - src/package-converter.js | 16 ++++++++-------- src/rebuild.js | 7 +++++-- src/search.js | 7 ++++++- src/star.js | 3 +-- src/stars.js | 9 +++++++-- src/text-mate-theme.js | 7 +++---- src/tree.js | 6 ++---- src/upgrade.js | 3 +-- src/view.js | 7 ++++++- 20 files changed, 99 insertions(+), 65 deletions(-) diff --git a/src/apm-cli.js b/src/apm-cli.js index 5a52d5fd..347728c6 100644 --- a/src/apm-cli.js +++ b/src/apm-cli.js @@ -1,7 +1,6 @@ const {spawn} = require('child_process'); const path = require('path'); -const _ = require('underscore-plus'); const colors = require('colors'); const npm = require('npm'); const yargs = require('yargs'); @@ -215,12 +214,7 @@ module.exports = { if (callbackCalled) { return; } callbackCalled = true; if (error != null) { - let message; - if (_.isString(error)) { - message = error; - } else { - message = error.message != null ? error.message : error; - } + const message = typeof error === 'string' ? error : error.message ?? error; if (message === 'canceled') { // A prompt was canceled so just log an empty line diff --git a/src/ci.js b/src/ci.js index 95b6da5a..22631817 100644 --- a/src/ci.js +++ b/src/ci.js @@ -2,7 +2,6 @@ const path = require('path'); const fs = require('./fs'); const yargs = require('yargs'); -const _ = require('underscore-plus'); const config = require('./apm'); const Command = require('./command'); @@ -54,7 +53,11 @@ but cannot be used to install new packages or dependencies.\ fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); const installOptions = {env, streaming: options.argv.verbose}; diff --git a/src/command.js b/src/command.js index 034b186d..942cf35d 100644 --- a/src/command.js +++ b/src/command.js @@ -1,7 +1,6 @@ const child_process = require('child_process'); const path = require('path'); -const _ = require('underscore-plus'); const semver = require('semver'); const config = require('./apm'); const git = require('./git'); @@ -58,7 +57,7 @@ class Command { sanitizePackageNames(packageNames) { packageNames ??= []; packageNames = packageNames.map(packageName => packageName.trim()); - return _.compact(_.uniq(packageNames)); + return Array.from(new Set(packageNames)).filter(Boolean); } logSuccess() { diff --git a/src/config.js b/src/config.js index 149e87c1..0efa78fe 100644 --- a/src/config.js +++ b/src/config.js @@ -1,6 +1,5 @@ const path = require('path'); -const _ = require('underscore-plus'); const yargs = require('yargs'); const apm = require('./apm'); const Command = require('./command'); @@ -37,7 +36,11 @@ Usage: ppm config set let configArgs = ['--globalconfig', apm.getGlobalConfigPath(), '--userconfig', apm.getUserConfigPath(), 'config']; configArgs = configArgs.concat(options.argv._); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: apm.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: apm.getRustupHomeDirPath() + }; const configOptions = {env}; return new Promise((resolve, _reject) => diff --git a/src/dedupe.js b/src/dedupe.js index c3444b89..3670f74d 100644 --- a/src/dedupe.js +++ b/src/dedupe.js @@ -1,7 +1,6 @@ const path = require('path'); -const _ = require('underscore-plus'); const yargs = require('yargs'); const config = require('./apm'); @@ -54,7 +53,11 @@ This command is experimental.\ fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); const dedupeOptions = {env}; diff --git a/src/develop.js b/src/develop.js index f80664c8..e63df66b 100644 --- a/src/develop.js +++ b/src/develop.js @@ -2,7 +2,6 @@ const fs = require('fs'); const path = require('path'); -const _ = require('underscore-plus'); const yargs = require('yargs'); const config = require('./apm'); @@ -81,16 +80,17 @@ cmd-shift-o to run the package out of the newly cloned repository.\ } installDependencies(packageDirectory, options) { - process.chdir(packageDirectory); - const installOptions = _.clone(options); + process.chdir(packageDirectory); - return new Install().run(installOptions); + return new Install().run({...options}); } linkPackage(packageDirectory, options) { - const linkOptions = _.clone(options); + const linkOptions = { + ...options, + commandArgs: [packageDirectory, '--dev'] + }; - linkOptions.commandArgs = [packageDirectory, '--dev']; return new Link().run(linkOptions); } diff --git a/src/disable.js b/src/disable.js index 428313cd..36ddd2d4 100644 --- a/src/disable.js +++ b/src/disable.js @@ -59,13 +59,13 @@ Disables the named package(s).\ try { const installedPackages = await this.getInstalledPackages(); const installedPackageNames = Array.from(installedPackages).map((pkg) => pkg.name); - const uninstalledPackageNames = _.difference(packageNames, installedPackageNames); - if (uninstalledPackageNames.length > 0) { - console.log(`Not Installed:\n ${uninstalledPackageNames.join('\n ')}`); + const notInstalledPackageNames = packageNames.filter(elem => !installedPackageNames.includes(elem)); + if (notInstalledPackageNames.length > 0) { + console.log(`Not Installed:\n ${notInstalledPackageNames.join('\n ')}`); } // only installed packages can be disabled - packageNames = _.difference(packageNames, uninstalledPackageNames); + packageNames = packageNames.filter(elem => installedPackageNames.includes(elem)); if (packageNames.length === 0) { return "Please specify a package to disable"; //errors as return values atm @@ -73,7 +73,7 @@ Disables the named package(s).\ const keyPath = '*.core.disabledPackages'; const disabledPackages = _.valueForKeyPath(settings, keyPath) ?? []; - const result = _.union(disabledPackages, packageNames); + const result = Array.from(new Set([...disabledPackages, ...packageNames])); _.setValueForKeyPath(settings, keyPath, result); try { diff --git a/src/enable.js b/src/enable.js index 57a38c75..2a5e9646 100644 --- a/src/enable.js +++ b/src/enable.js @@ -42,19 +42,19 @@ Enables the named package(s).\ const keyPath = '*.core.disabledPackages'; const disabledPackages = _.valueForKeyPath(settings, keyPath) ?? []; - const errorPackages = _.difference(packageNames, disabledPackages); + const errorPackages = packageNames.filter(elem => !disabledPackages.includes(elem)); if (errorPackages.length > 0) { console.log(`Not Disabled:\n ${errorPackages.join('\n ')}`); } - // can't enable a package that isn't disabled - packageNames = _.difference(packageNames, errorPackages); + // can only enable a package that is disabled + packageNames = packageNames.filter(elem => disabledPackages.includes(elem)); if (packageNames.length === 0) { return "Please specify a package to enable"; //errors as retval atm } - const result = _.difference(disabledPackages, packageNames); + const result = disabledPackages.filter(elem => !packageNames.includes(elem)); _.setValueForKeyPath(settings, keyPath, result); try { diff --git a/src/featured.js b/src/featured.js index 5aeaf88b..1bf50dc6 100644 --- a/src/featured.js +++ b/src/featured.js @@ -40,8 +40,14 @@ List the Pulsar packages and themes that are currently featured.\ const body = response.body ?? []; if (response.statusCode === 200) { let packages = body.filter(pack => pack?.releases != null); - packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); - packages = _.sortBy(packages, 'name'); + packages = packages.map(({readme, metadata, downloads, stargazers_count}) => ({ + ...metadata, + readme, + downloads, + stargazers_count + })); + + packages.sort((left, right) => left.name.localeCompare(right.name)); return packages; } diff --git a/src/install.js b/src/install.js index 879eb0a0..0540527f 100644 --- a/src/install.js +++ b/src/install.js @@ -2,7 +2,6 @@ const assert = require('assert'); const path = require('path'); -const _ = require('underscore-plus'); const async = require('async'); const CSON = require('season'); const yargs = require('yargs'); @@ -82,7 +81,11 @@ package names to install with optional versions using the fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); const installOptions = {env}; @@ -194,7 +197,11 @@ Run ppm -v after installing Git to see what version has been detected.\ fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); const installOptions = {env}; @@ -363,7 +370,10 @@ Run ppm -v after installing Git to see what version has been detected.\ // // return value - A Promise that rejects with an error or resolves without a value async installPackageDependencies(options) { - options = _.extend({}, options, {installGlobally: false}); + options = { + ...options, + installGlobally: false + }; const commands = []; const object = this.getPackageDependencies(options.cwd); for (let name in object) { @@ -446,7 +456,11 @@ Run ppm -v after installing Git to see what version has been detected.\ fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); const buildOptions = {env}; @@ -717,7 +731,7 @@ with Pulsar will be used.\ const iteratee = async fn => await fn(); try { let installedPackagesInfo = await async.mapSeries(commands, iteratee); - installedPackagesInfo = _.compact(installedPackagesInfo); + installedPackagesInfo = installedPackagesInfo.filter(Boolean); installedPackagesInfo = installedPackagesInfo.filter((item, idx) => packageNames[idx] !== "."); if (options.argv.json) { console.log(JSON.stringify(installedPackagesInfo, null, " ")); } } catch (error) { diff --git a/src/list.js b/src/list.js index 37629525..ccd35bbf 100644 --- a/src/list.js +++ b/src/list.js @@ -1,7 +1,6 @@ const path = require('path'); -const _ = require('underscore-plus'); const CSON = require('season'); const yargs = require('yargs'); diff --git a/src/package-converter.js b/src/package-converter.js index 502641d3..802999f5 100644 --- a/src/package-converter.js +++ b/src/package-converter.js @@ -100,21 +100,21 @@ class PackageConverter { settings.shellVariables = shellVariables; } - const editorProperties = _.compactObject({ + const editorPropertyEntries = Object.entries({ commentStart: _.valueForKeyPath(settings, 'shellVariables.TM_COMMENT_START'), commentEnd: _.valueForKeyPath(settings, 'shellVariables.TM_COMMENT_END'), increaseIndentPattern: settings.increaseIndentPattern, decreaseIndentPattern: settings.decreaseIndentPattern, foldEndPattern: settings.foldingStopMarker, completions: settings.completions - }); - if (!_.isEmpty(editorProperties)) { return {editor: editorProperties}; } + }).filter(([_, value]) => value != null); + if (editorPropertyEntries.length > 0) { return {editor: Object.fromEntries(editorPropertyEntries)}; } } readFileSync(filePath) { - if (_.contains(this.plistExtensions, path.extname(filePath))) { + if (this.plistExtensions.includes(path.extname(filePath))) { return plist.parse(fs.readFileSync(filePath, 'utf8')); - } else if (_.contains(['.json', '.cson'], path.extname(filePath))) { + } else if (['.json', '.cson'].includes(path.extname(filePath))) { return CSON.readFileSync(filePath); } } @@ -134,9 +134,9 @@ class PackageConverter { const destinationPath = path.join(destinationDir, destinationName); let contents; - if (_.contains(this.plistExtensions, path.extname(sourcePath))) { + if (this.plistExtensions.includes(path.extname(sourcePath))) { contents = plist.parse(fs.readFileSync(sourcePath, 'utf8')); - } else if (_.contains(['.json', '.cson'], path.extname(sourcePath))) { + } else if (['.json', '.cson'].includes(path.extname(sourcePath))) { contents = CSON.readFileSync(sourcePath); } @@ -244,7 +244,7 @@ class PackageConverter { const value = properties[key]; preferencesBySelector[selector] ??= {}; preferencesBySelector[selector][key] = preferencesBySelector[selector][key] != null - ? _.extend(value, preferencesBySelector[selector][key]) + ? { ...value, ...preferencesBySelector[selector][key] } : value; } } diff --git a/src/rebuild.js b/src/rebuild.js index dbac1cf8..38d0d947 100644 --- a/src/rebuild.js +++ b/src/rebuild.js @@ -1,7 +1,6 @@ const path = require('path'); -const _ = require('underscore-plus'); const yargs = require('yargs'); const config = require('./apm'); @@ -43,7 +42,11 @@ All the modules will be rebuilt if no module names are specified.\ fs.makeTreeSync(this.atomDirectory); - const env = _.extend({}, process.env, {HOME: this.atomNodeDirectory, RUSTUP_HOME: config.getRustupHomeDirPath()}); + const env = { + ...process.env, + HOME: this.atomNodeDirectory, + RUSTUP_HOME: config.getRustupHomeDirPath() + }; this.addBuildEnvVars(env); return new Promise((resolve, reject) => diff --git a/src/search.js b/src/search.js index 4b34d4cd..130d92b5 100644 --- a/src/search.js +++ b/src/search.js @@ -49,7 +49,12 @@ Search for packages/themes.\ if (response.statusCode === 200) { let packages = body.filter(pack => pack?.releases?.latest != null); - packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); + packages = packages.map(({readme, metadata, downloads, stargazers_count}) => ({ + ...metadata, + readme, + downloads, + stargazers_count + })); packages = packages.filter(({name, version}) => !isDeprecatedPackage(name, version)); return packages; } diff --git a/src/star.js b/src/star.js index cc018ec3..37b9758e 100644 --- a/src/star.js +++ b/src/star.js @@ -1,7 +1,6 @@ const path = require('path'); -const _ = require('underscore-plus'); const async = require('async'); const CSON = require('season'); const yargs = require('yargs'); @@ -77,7 +76,7 @@ Run \`ppm stars\` to see all your starred packages.\ } } - return _.uniq(installedPackages); + return Array.from(new Set(installedPackages)); } async run(options) { diff --git a/src/stars.js b/src/stars.js index e6fd3b15..b1619108 100644 --- a/src/stars.js +++ b/src/stars.js @@ -52,8 +52,13 @@ List or install starred Atom packages and themes.\ const body = response.body ?? []; if (response.statusCode === 200) { let packages = body.filter(pack => pack?.releases?.latest != null); - packages = packages.map(({readme, metadata, downloads, stargazers_count}) => _.extend({}, metadata, {readme, downloads, stargazers_count})); - packages = _.sortBy(packages, 'name'); + packages = packages.map(({readme, metadata, downloads, stargazers_count}) => ({ + ...metadata, + readme, + downloads, + stargazers_count + })); + packages.sort((left, right) => left.name.localeCompare(right.name)); return packages; } diff --git a/src/text-mate-theme.js b/src/text-mate-theme.js index e38e96ae..62b0d542 100644 --- a/src/text-mate-theme.js +++ b/src/text-mate-theme.js @@ -1,5 +1,4 @@ -const _ = require('underscore-plus'); const plist = require('plist'); const {ScopeSelector, ready} = require('second-mate'); @@ -180,9 +179,9 @@ atom-text-editor.is-focused .line.cursor-line`, if (fontStyle) { const fontStyles = fontStyle.split(/\s+/); - if (_.contains(fontStyles, 'bold')) { properties['font-weight'] = 'bold'; } - if (_.contains(fontStyles, 'italic')) { properties['font-style'] = 'italic'; } - if (_.contains(fontStyles, 'underline')) { properties['text-decoration'] = 'underline'; } + if (fontStyles.includes('bold')) { properties['font-weight'] = 'bold'; } + if (fontStyles.includes('italic')) { properties['font-style'] = 'italic'; } + if (fontStyles.includes('underline')) { properties['text-decoration'] = 'underline'; } } if (foreground) { properties['color'] = this.translateColor(foreground); } diff --git a/src/tree.js b/src/tree.js index 5fbee28d..ff79910b 100644 --- a/src/tree.js +++ b/src/tree.js @@ -1,9 +1,7 @@ -const _ = require('underscore-plus'); - -module.exports = (items, options, callback) => { +module.exports = (items, options, callback) => { //TODO clean up signature and usage options ??= {}; - if (_.isFunction(options)) { + if (options instanceof Function) { callback = options; options = {}; } diff --git a/src/upgrade.js b/src/upgrade.js index b5c5f442..62ff7443 100644 --- a/src/upgrade.js +++ b/src/upgrade.js @@ -1,7 +1,6 @@ const path = require('path'); -const _ = require('underscore-plus'); const async = require('async'); const yargs = require('yargs'); const read = require('read'); @@ -171,7 +170,7 @@ available updates.\ }; let updates = await async.mapLimit(packages, 10, getLatestVersionOrSha); - updates = _.filter(updates, update => (update.latestVersion != null) || (update.sha != null)); + updates = updates.filter(update => (update.latestVersion != null) || (update.sha != null)); updates.sort((updateA, updateB) => updateA.pack.name.localeCompare(updateB.pack.name)); return updates; } diff --git a/src/view.js b/src/view.js index 35cbb7fc..e169d095 100644 --- a/src/view.js +++ b/src/view.js @@ -84,7 +84,12 @@ View information about a package/theme.\ const version = await this.getLatestCompatibleVersion(body, options); const {name, readme, downloads, stargazers_count} = body; const metadata = body.versions?.[version] ?? {name}; - const pack = _.extend({}, metadata, {readme, downloads, stargazers_count}); + const pack = { + ...metadata, + readme, + downloads, + stargazers_count + }; return pack; } From 310756f0a6642b802cd835a59cd050218cea54ec Mon Sep 17 00:00:00 2001 From: 2colours Date: Sun, 22 Dec 2024 02:55:49 +0100 Subject: [PATCH 3/3] Review related fixes --- src/command.js | 2 +- src/develop.js | 2 +- src/disable.js | 2 +- src/install.js | 5 +++-- src/search.js | 2 +- src/star.js | 2 +- src/unstar.js | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/command.js b/src/command.js index 942cf35d..6fb0bb49 100644 --- a/src/command.js +++ b/src/command.js @@ -57,7 +57,7 @@ class Command { sanitizePackageNames(packageNames) { packageNames ??= []; packageNames = packageNames.map(packageName => packageName.trim()); - return Array.from(new Set(packageNames)).filter(Boolean); + return Array.from(new Set(packageNames)).filter(Boolean); // Array of only unique truthy values } logSuccess() { diff --git a/src/develop.js b/src/develop.js index e63df66b..ebb67f98 100644 --- a/src/develop.js +++ b/src/develop.js @@ -82,7 +82,7 @@ cmd-shift-o to run the package out of the newly cloned repository.\ installDependencies(packageDirectory, options) { process.chdir(packageDirectory); - return new Install().run({...options}); + return new Install().run({...options}); // pass options shallow-copied - protects somewhat from mutations } linkPackage(packageDirectory, options) { diff --git a/src/disable.js b/src/disable.js index 36ddd2d4..d6f24aff 100644 --- a/src/disable.js +++ b/src/disable.js @@ -73,7 +73,7 @@ Disables the named package(s).\ const keyPath = '*.core.disabledPackages'; const disabledPackages = _.valueForKeyPath(settings, keyPath) ?? []; - const result = Array.from(new Set([...disabledPackages, ...packageNames])); + const result = Array.from(new Set([...disabledPackages, ...packageNames])); // create a union of the two arrays (no duplicates) //TODO use proper set operations when the runtime allows them _.setValueForKeyPath(settings, keyPath, result); try { diff --git a/src/install.js b/src/install.js index 0540527f..dc3ade4e 100644 --- a/src/install.js +++ b/src/install.js @@ -223,8 +223,9 @@ Run ppm -v after installing Git to see what version has been detected.\ retries: 4 }; const response = await request.get(requestSettings).catch(error => { - const message = request.getErrorMessage(body, error); - throw `Request for package information failed: ${message}`; + let message = `Request for package information failed: ${error.message}`; + if (error.status) { message += ` (${error.status})`; } + throw message; }); const body = response.body ?? {}; if (response.statusCode !== 200) { diff --git a/src/search.js b/src/search.js index 130d92b5..c4ea798d 100644 --- a/src/search.js +++ b/src/search.js @@ -59,7 +59,7 @@ Search for packages/themes.\ return packages; } - const message = request.getErrorMessage(body, error); + const message = request.getErrorMessage(body, null); throw `Searching packages failed: ${message}`; } diff --git a/src/star.js b/src/star.js index 37b9758e..e5f7251b 100644 --- a/src/star.js +++ b/src/star.js @@ -76,7 +76,7 @@ Run \`ppm stars\` to see all your starred packages.\ } } - return Array.from(new Set(installedPackages)); + return Array.from(new Set(installedPackages)); // Unique only array } async run(options) { diff --git a/src/unstar.js b/src/unstar.js index cb0abeaf..aef7c344 100644 --- a/src/unstar.js +++ b/src/unstar.js @@ -39,7 +39,7 @@ Run \`ppm stars\` to see all your starred packages.\ const body = response.body ?? {}; if (response.statusCode !== 204) { this.logFailure(); - const message = request.getErrorMessage(body, error); + const message = request.getErrorMessage(body, null); throw `Unstarring package failed: ${message}`; }