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.

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

Check parent builder object for enabled status #142

Merged
merged 1 commit into from Jan 30, 2017
Merged

Conversation

Qix-
Copy link
Member

@Qix- Qix- commented Jan 30, 2017

This fixes an issue where child builders do not propagate their enabled statuses to the parents and vice versa.

For example, prior to this PR:

var chalk = require('chalk');

console.log(chalk.enabled); // true

var red = chalk.red;
console.log(red.enabled); // true

chalk.enabled = false;

console.log(red.enabled); // true  (should be false!)

This PR fixes that; getting/setting .enabled on any builder (anything stemming from the root chalk object) now reflects the .enabled value on chalk as a whole.

@Qix-
Copy link
Member Author

Qix- commented Jan 30, 2017

Tbh we should just fit this in with #140 and release with the major. It's technically a bug fix but I don't want to take any chances on someone relying on this behavior of the .enabled property.

If you are relying on it, you're using it wrong; expect it to break regardless of this PR being merged or not.

@sindresorhus
Copy link
Member

Tbh we should just fit this in with #140 and release with the major.

Agreed. Don't want to risk it.


Wouldn't it be more efficient and better to put .enabled on the Chalk prototype instead so it's just inherited?

@Qix-
Copy link
Member Author

Qix- commented Jan 30, 2017

@sindresorhus No because we don't have a reference to the parent builder.

The following modifications fail.

'use strict';
var escapeStringRegexp = require('escape-string-regexp');
var ansiStyles = require('ansi-styles');
var supportsColor = require('supports-color');

var defineProps = Object.defineProperties;
var isSimpleWindowsTerm = process.platform === 'win32' && !/^xterm/i.test(process.env.TERM);

function Chalk(options) {
	// detect mode if not set manually
	this._enabled = !options || options.enabled === undefined ? supportsColor : options.enabled;
}

// use bright blue on Windows as the normal blue color is illegible
if (isSimpleWindowsTerm) {
	ansiStyles.blue.open = '\u001b[94m';
}

var styles = {};

Object.keys(ansiStyles).forEach(function (key) {
	ansiStyles[key].closeRe = new RegExp(escapeStringRegexp(ansiStyles[key].close), 'g');

	styles[key] = {
		get: function () {
			return build.call(this, this._styles ? this._styles.concat(key) : [key]);
		}
	};
});

styles.enabled = {
	enumerable: true,
	get: function () {
		return this._enabled;
	},
	set: function (v) {
		this._enabled = v;
	}
};

// eslint-disable-next-line func-names
var proto = defineProps(function chalk() {}, styles);

function build(_styles) {
	var builder = function () {
		return applyStyle.apply(builder, arguments);
	};

	builder._styles = _styles;

	// __proto__ is used because we must return a function, but there is
	// no way to create a function with a different prototype.
	/* eslint-disable no-proto */
	builder.__proto__ = proto;

	return builder;
}

function applyStyle() {
	// support varags, but simply cast to string in case there's only one arg
	var args = arguments;
	var argsLen = args.length;
	var str = argsLen !== 0 && String(arguments[0]);

	if (argsLen > 1) {
		// don't slice `arguments`, it prevents v8 optimizations
		for (var a = 1; a < argsLen; a++) {
			str += ' ' + args[a];
		}
	}

	if (!this.enabled || !str) {
		return str;
	}

	var nestedStyles = this._styles;
	var i = nestedStyles.length;

	// Turns out that on Windows dimmed gray text becomes invisible in cmd.exe,
	// see https://github.com/chalk/chalk/issues/58
	// If we're on Windows and we're dealing with a gray color, temporarily make 'dim' a noop.
	var originalDim = ansiStyles.dim.open;
	if (isSimpleWindowsTerm && (nestedStyles.indexOf('gray') !== -1 || nestedStyles.indexOf('grey') !== -1)) {
		ansiStyles.dim.open = '';
	}

	while (i--) {
		var code = ansiStyles[nestedStyles[i]];

		// Replace any instances already present with a re-opening code
		// otherwise only the part of the string until said closing code
		// will be colored, and the rest will simply be 'plain'.
		str = code.open + str.replace(code.closeRe, code.open) + code.close;

		// Close the styling before a linebreak and reopen
		// after next line to fix a bleed issue on macOS
		// https://github.com/chalk/chalk/pull/92
		str = str.replace(/\r?\n/g, code.close + '$&' + code.open);
	}

	// Reset the original 'dim' if we changed it to work around the Windows dimmed gray issue.
	ansiStyles.dim.open = originalDim;

	return str;
}

defineProps(Chalk.prototype, styles);

module.exports = new Chalk();
module.exports.styles = ansiStyles;
module.exports.supportsColor = supportsColor;

@sindresorhus
Copy link
Member

sindresorhus commented Jan 30, 2017

Alright. I still think each style should be an instance of Chalk. So const blue = chalk.blue; blue instance of chalk === true, but we can work on cleaning up later.

Feel free to merge this or integrate into your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants