Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

Rework profile.js to use shellInitScriptChooser() #51

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
125 changes: 117 additions & 8 deletions lib/setup/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,110 @@ var appendScript = function(name, options) {
});
};

/**
* Chooses the correct shell init script to insert avn initialization string to,
* based on the following criteria:
* 1. Append to the one that's present, if the other doesn't exist,
* 2. If both exist, append to the one that is sourced by the other, or
* 3. If both exist, but neither sources the other,
* append to .bashrc if platform is 'linux',
* or append to .bash_profile if platform is 'darwin' (i.e. macOS)
*
* @private
* @function setup.profile.~shellInitScriptChooser
* @return {Promise}
*/
var shellInitScriptChooser = function() {
Copy link
Owner

Choose a reason for hiding this comment

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

Since this is bash specific, let's name it as such.

var bashrcFile = path.join(process.env.HOME, '.bashrc');
var bashProfileFile = path.join(process.env.HOME, '.bash_profile');
var bashrcExists;
var bashProfileExists;

// Check if .bashrc exists
var bashrcPromise = new Promise(function(resolve) {
fs.access(bashrcFile, function(err) {
bashrcExists = !err;
resolve();
});
});
Copy link
Owner

Choose a reason for hiding this comment

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

Since we have mz, you should be able to just do:

var bashrcPromise = fs.access(bashrcFile).then(function() { bashrcExists = true; });


// Check if .bash_profile exists
var bashProfilePromise = new Promise(function(resolve) {
fs.access(bashProfileFile, function(err) {
bashProfileExists = !err;
resolve();
});
});
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above comment.


var promise = Promise.all([bashrcPromise, bashProfilePromise])
.then(function() {
if ((bashrcExists || bashProfileExists) &&
!(bashrcExists && bashProfileExists)) { // ^ == bitwise XOR
return (bashrcExists && bashrcFile) ||
(bashProfileExists && bashProfileFile);
}

if (bashrcExists && bashProfileExists) {
var innerPromise = new Promise(function(resolve) {
Copy link
Owner

Choose a reason for hiding this comment

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

Again, no need for innerPromise via mz.

fs.readFile(bashProfileFile, 'utf8', function(err, data) {
// RegExp to detect "~/.bashrc",
// "$HOME/.bashrc", or "/home/user/.bashrc"
var bashrcRe = new RegExp(' ?source +(\\\\\\n)? *(~\/.bashrc|' +
'\"?\\$HOME\/.bashrc\"?|[\'\"]?\/home\/\\w+\/.bashrc[\'\"]?)');

if (bashrcRe.test(data)) {
resolve(bashrcFile);
}
else {
resolve();
}
});
})
.then(function(appendFile) {
if (appendFile) {
return appendFile;
}

var innerInnerPromise = new Promise(function(resolve) {
Copy link
Owner

Choose a reason for hiding this comment

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

mz

fs.readFile(bashrcFile, 'utf8', function(err, data) {
// RegExp to detect "~/.bash_profile",
// "$HOME/.bash_profile", or "/home/user/.bash_profile"
var bashProfileRe = new RegExp(' ?source +(\\\\\\n)? *' +
'(~\/.bash_profile|\"?\\$HOME\/.bash_profile\"?|' +
'[\'\"]?\/home\/\\w+\/.bash_profile[\'\"]?)');

if (bashProfileRe.test(data)) {
resolve(bashProfileFile);
}
else {
resolve();
}
});
});

return innerInnerPromise;
})
.then(function(appendFile) {
if (appendFile) {
return appendFile;
}

// Neither .bashrc nor .bash_profile sources the other
var platformsToInitScript = {
linux: bashrcFile,
darwin: bashProfileFile,
};

return platformsToInitScript[process.platform];
Copy link
Owner

Choose a reason for hiding this comment

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

There should probably be a fallback here in case the platform is not in this list.

});

return innerPromise;
}
});

return promise;
};

/**
* Update `~/.bash_profile` and `.zshrc` shell profile files.
*
Expand All @@ -77,33 +181,38 @@ var appendScript = function(name, options) {
module.exports.update = function() {
var handled = false;
var changed = false;
var appendFile;

var promise = Promise.map(['.bash_profile', '.zshrc'], function(name) {
return appendScript(name);
var promise = shellInitScriptChooser()
.then(function(initScript) {
appendFile = initScript;
return appendScript(appendFile);
})
.each(function(result) {
.then(function(result) {
if (result) {
handled = true;
changed = changed || (result === 'change');
}
});
})

promise = promise.then(function() {
.then(function() {
if (!handled) {
return appendScript('.bash_profile', { force: true });
return appendScript(appendFile, { force: true });
}
})
.then(function(result) {
if (result) {
handled = true;
changed = changed || (result === 'change');
}
});
})

return promise.then(function() {
.then(function() {
if (changed) {
console.log('%s: %s', chalk.bold.magenta('avn'),
chalk.bold.cyan('restart your terminal to start using avn'));
}
});

return promise;
Copy link
Owner

Choose a reason for hiding this comment

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

This should not change — the returned promise resolution would occur before the final log message is output (which is not desirable & shouldn't be required for this PR).

};