Fix colors not working in grunt

[paladox]

In https://github.com/Marak/colors.js they moved colors file into lib folder but you doint call that any more you call colors/lib/colors

There doc seems to not be correct since doing colors in my main folder and colors installed in node_modules does not work I have to add colors/lib/colors.

[paladox]

@shama and @vladikoff please review and merge.

[paladox]

Tested 0.6.2 of colors and doing just require colors works so yes this fixes it.

[paladox]

We should update this package here and then update in grunt and release a minor version please.

[paladox]

Looks like this will work.

[shama]

I don't understand how this fixes the issue. Their main in their package.json points to "lib". By Node.js convention, it will load an index.js file. So require('colors') should be the same as require('colors/lib/index')

[shama]

Oh I think I see why. "lib" as main will first look for the module's name lib/color.js before lib/index.js. Since they have a file called lib/color.js, it's loading that which doesnt include the string prototype extensions.

[shama]

Never mind, I take that back. require('colors') is loading node_modules/colors/lib/index.js.

@paladox Which version of Node.js are you running?

[paladox]

@shama I'm running nodejs 4.4.

And not really. There docs are wrong because there is a patch that fixes where the main task is.

Marak/colors.js#138

Since I tried on my local machine.

Since in package.json in colors there main task is lib but there index.js and colors.js so it will just load colors.js and cause it to fail.

[shama]

Strange, I'm on Node.js v4.4.1 on OSX and it is loading index.js for me.

[paladox]

Oh. Did you do npm install colour which installs the latest and npm install colors@0.6.2.

[shama]

I npm i grunt@1.0.1, added a debugger on ./node_modules/colors/lib/index.js then ran devtool ./node_modules/.bin/grunt on this Gruntfile:

module.exports = function (grunt) {
  grunt.registerTask('default', function () {
    console.log('testing'.green)
  })
}

[paladox]

@shama oh doing it this way works on Windows. Doing it the other way dosent. But if it works for you on a Mac which is Linux why doesn't it work for the other such as the test I did on wikimedia ci.

Maybe doing it the old way works for some and dosent for some. Doing it this way fixers it for everyone. Since it looks like mostly the same code you use to get to show the colour only difference is under the hood plus maybe other changes.

[shama]

I don't want to presume this is a fix. I would prefer to understand why this fixes the issue you were running into. That way we don't repeat the mistake or end up causing another mistake by merging this. Also if prototype extensions are broken for everyone, I'd expect to see more issues about it. So I think we are overlooking something here.

[paladox]

@shama doing just var colors = require('colors'); using colors 0.6.2 worked for me but doing it on colors 1.1.2 didn't doing var colors = require('colors/lib/index'); worked on colors 1.1.2.

Maybe we can revert colors back to 0.6.2 and re update colors to 1.1.2 in a pull to try and figure out why this issue is happening.

[shama]

@paladox I understand but keep in mind this gets downloaded 1.7M times a month. I prefer to know why this fixes it before blindly merging and releasing. Is there something about the version/distro of Linux of your CI server? The version of Node.js/npm different on your CI server? Is there something else your CI server is doing? Is it using an old cached version of colors? Maybe the escape sequences are being rendered differently with that Jenkins theme?

Are you able to ssh into your CI server and see if it's displaying the colors in your terminal? Change up the Jenkins theme and see if that's the issue? npm cache clean everything and make sure you're getting the latest modules. I'm not able to reproduce your issue so I need help from you.

I'm hesitant to do require('colors/lib/index') as many authors don't consider moving libraries around in their packages breaking changes. So if we merge this, we'll need to hard tag that dependency.

[paladox]

@shama oh, I carn't ssh into it. I can only upload patches to gerrit and then it is sent for testing through jenkins. But maybe yes it could be cached, but the cache would have cleared now. But if that was it then why carn't I do require('colors'); on windows. It works with colors 0.6.2 but not with 1.1.2.

[paladox]

@shama but using colors 0.6.2 on Windows works but using 1.1.2 doesn't.

[paladox]

@shama I tested colors 0.6.2 with grunt and works but 1.1.2 doesen't so it is colors and this patch doesn't fix it.

[paladox]

@shama it looks like index.js loads for me it just doesn't show any colour unless I do this.

[paladox]

I found where the problem is coming from.

Its coming from supports-colors.js from in colors package.

The code causing it

if (argv.indexOf('--no-color') !== -1 ||
argv.indexOf('--color=false') !== -1) {
return false;
}

if (argv.indexOf('--color') !== -1 ||
argv.indexOf('--color=true') !== -1 ||
argv.indexOf('--color=always') !== -1) {
return true;
}

Replacing with

return true;

made the colour work.

[paladox]

@shama I dugged deeper and found we need to pass this --color=true to grunt otherwise color wont work

Doing grunt test --color=true worked for me.

[paladox]

I submitted a pull request here Marak/colors.js#157