Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate trivial underscore #147

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Eliminate trivial underscore #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28a3f0f
Changing own request.js to async interface
2colours Aug 31, 2024
358d44e
Remove uses of underscore where it's obvious
2colours Dec 1, 2024
310756f
Review related fixes
2colours Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions src/apm-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions src/ci.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanting to add, since this one made me a bit nervous, I've tested to confirm this behavior will match what underscore-plus does on it's particular version.

Ensuring that variables are overriden properly based on these news ones assigned. Smart use, and one of my favourites, of spread syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wanted to be sure and specifically checked MDN - it's guaranteed that what comes later overrides earlier values. I really share the mixed feelings here: if everybody knew this, probably we wouldn't fear it but still, it doesn't seem trivial...

HOME: this.atomNodeDirectory,
RUSTUP_HOME: config.getRustupHomeDirPath()
};
this.addBuildEnvVars(env);

const installOptions = {env, streaming: options.argv.verbose};
Expand Down
3 changes: 1 addition & 2 deletions src/command.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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); // Array of only unique truthy values
}

logSuccess() {
Expand Down
7 changes: 5 additions & 2 deletions src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

const path = require('path');
const _ = require('underscore-plus');
const yargs = require('yargs');
const apm = require('./apm');
const Command = require('./command');
Expand Down Expand Up @@ -37,7 +36,11 @@ Usage: ppm config set <key> <value>
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) =>
Expand Down
7 changes: 5 additions & 2 deletions src/dedupe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

const path = require('path');

const _ = require('underscore-plus');
const yargs = require('yargs');

const config = require('./apm');
Expand Down Expand Up @@ -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};
Expand Down
54 changes: 24 additions & 30 deletions src/develop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
const fs = require('fs');
const path = require('path');

const _ = require('underscore-plus');
const yargs = require('yargs');

const config = require('./apm');
Expand Down Expand Up @@ -43,31 +42,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) {
Expand All @@ -87,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}); // pass options shallow-copied - protects somewhat from mutations
}

linkPackage(packageDirectory, options) {
const linkOptions = _.clone(options);
const linkOptions = {
...options,
commandArgs: [packageDirectory, '--dev']
};

linkOptions.commandArgs = [packageDirectory, '--dev'];
return new Link().run(linkOptions);
}

Expand Down
10 changes: 5 additions & 5 deletions src/disable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,21 @@ 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
}

const keyPath = '*.core.disabledPackages';
const disabledPackages = _.valueForKeyPath(settings, keyPath) ?? [];
const result = _.union(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 {
Expand Down
8 changes: 4 additions & 4 deletions src/enable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
34 changes: 18 additions & 16 deletions src/featured.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,31 @@ 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`,
json: true
};
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);
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}`);
}));
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}) => ({
...metadata,
readme,
downloads,
stargazers_count
}));

packages.sort((left, right) => left.name.localeCompare(right.name));
return packages;
}

const message = request.getErrorMessage(body, error);
throw `Requesting packages failed: ${message}`;
}

async getAllFeaturedPackages(atomVersion) {
Expand Down
61 changes: 35 additions & 26 deletions src/install.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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};
Expand All @@ -209,32 +216,27 @@ 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 => {
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) {
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?
Expand Down Expand Up @@ -369,7 +371,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) {
Expand Down Expand Up @@ -452,7 +457,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};
Expand Down Expand Up @@ -723,7 +732,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) {
Expand Down
Loading
Loading