Skip to content
This repository was archived by the owner on Dec 6, 2023. It is now read-only.
146 changes: 133 additions & 13 deletions lib/setup/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ var util = require('util');
var chalk = require('chalk');
var isNoEntry = require('../util/codes').isNoEntry;

var bashProfileFileName = '.bash_profile';
var bashrcFileName = '.bashrc';

Copy link
Owner

Choose a reason for hiding this comment

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

Given the comment about removing the branching condition, I think these globals can go away.

/**
* Generate the string to be included in the shell init script.
*
Expand Down Expand Up @@ -66,7 +69,104 @@ var appendScript = function(name, options) {
};

/**
* Update `~/.bash_profile` and `.zshrc` shell profile files.
* 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, bashrcFileName);
var bashProfileFile = path.join(process.env.HOME, bashProfileFileName);
var bashProfileNotExist;
var bashrcNotExist;

// Notice: will change fs.stat to fs.access in the future
// Reason: NodeJS 0.10 doesn't have fs.access (added in NodeJS 0.11.15)
Copy link
Owner

Choose a reason for hiding this comment

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

Since avn always uses the active node version, we'll likely support 0.10 for a long time. This comment could be removed.


// Check if .bash_profile exists
return fs.stat(bashProfileFile)
.catch(function(err) {
bashProfileNotExist = err;
})

// Check if .bashrc exists
.then(function() {
return fs.stat(bashrcFile)
.catch(function(err) {
bashrcNotExist = err;
});
})

.then(function() {
if (!bashProfileNotExist && !bashrcNotExist) {
return fs.readFile(bashProfileFile, 'utf8')
.then(function(data) {
// RegExp to detect "~/.bashrc",
// "$HOME/.bashrc", or "/home/user/.bashrc"
var bashrcRe = new RegExp(' ?source +(\\\\\\n)? *(~\/.bashrc|' +
Copy link
Owner

Choose a reason for hiding this comment

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

  • . is an alias of source
  • All space checks should probably use \s
  • Whitespace checks that can span a line should use the multiline workaround of [\s\S]
  • Checking for the full path seems unnecessary (~ vs $HOME vs /platform/specific/prefix) and error prone… for instance, I could write ${HOME} or ${HOME:/home}.

'\"?\\$HOME\/.bashrc\"?|[\'\"]?\/home\/\\w+\/.bashrc[\'\"]?)');

if (bashrcRe.test(data)) {
return bashrcFileName;
}
return;
})

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

return fs.readFile(bashrcFile, 'utf8')
.then(function(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)) {
return bashProfileFileName;
}
return;
});
})

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

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

return platformsToInitScript[process.platform] || bashProfileFileName;
});
}

else if (!bashProfileNotExist || !bashrcNotExist) { // XOR file existence
return (bashProfileNotExist && bashrcFileName) ||
(bashrcNotExist && bashProfileFileName);
}

else { // Default if neither exists
return bashProfileFileName;
}
});
};

/**
* Update `~/.bash_profile` and `.zshrc`, or `.bashrc` shell profile files.
*
* Adds the necessary commands to load `avn.sh` when a new shell is launched.
*
Expand All @@ -77,33 +177,53 @@ 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);
})
.each(function(result) {
if (result) {
handled = true;
changed = changed || (result === 'change');
var promise = shellInitScriptChooser()
.then(function(initScript) {
appendFile = initScript;

if (appendFile === bashProfileFileName) {
Copy link
Owner

Choose a reason for hiding this comment

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

What's the purpose of this if/else here?

Why can't it just update both .bash_profile and .zshrc or .bashrc and .zshrc? I know it's uncommon, but some people will use multiple shells, so if they have both config files, why not support them both?

return Promise.map([bashProfileFileName, '.zshrc'], function(name) {
return appendScript(name);
})
.each(function(result) {
if (result) {
handled = true;
changed = changed || (result === 'change');
}
});
}
});

promise = promise.then(function() {
else if (appendFile === bashrcFileName) {
return appendScript(bashrcFileName)
.then(function(result) {
if (result) {
handled = true;
changed = changed || (result === 'change');
}
});
}
})

.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).

};