fix: Replace colors with chalk to fix infinite loop.

[jwalton]

This replaces the colors library with chalk after the author of colors added an infinite loop to colors/lib/index.js as a protest.

The transition is pretty straightforward, although I had to skip a couple of tests where the ANSI gneerated by chalk was subtly different than the ANSI generated by the old cli-table library, or where the home-grown "truncate" function produced different output than chalk generated natively.

[Trott]

@jwalton If you're in the zone on this kind of thing, you might want to open a similar pull request to https://github.com/Automattic/cli-table, which also uses colors and which this package has as a dependency.

[jwalton]

@Trott cli-table is actually OK, because they have pegged colors to 1.0.3. cli-table3 is vulnerable because they use ^ in the version number, so you’ll automatically get the new version.

[Trott]

@Trott cli-table is actually OK, because they have pegged colors to 1.0.3. cli-table3 is vulnerable because they use ^ in the version number, so you’ll automatically get the new version.

While that's true, it's probably good to just get colors out of dependencies in widely-used packages in general. (But it's also not your obligation to do that or anything, of course.) The less use it has, the fewer downloads, the less likely people are to pick it for new projects.

[jwalton]

The other part of it is that cli-table3 exists because cli-table2 wasn't being maintained, and cli-table2 exists because cli-table wasn't being maintained. So I was a bit worried a PR wouldn't get looked at in cli-table (although it's last release is a month ago, and this project's last release was two years ago, so maybe I have that backwards.) Also also, cli-table is only a dev-dependency of this project, and only for backward compatibility testing. But, I'll go do a PR for cli-table, and see how it goes. :)

[DanielRuf]

@Trott @jwalton see also Marak/colors.js#285 (comment)

[JJ]

Much better than #251

[DanielRuf]

The transition is pretty straightforward, although I had to skip a couple of tests where the ANSI gneerated by chalk was subtly different than the ANSI generated by the old cli-table library, or where the home-grown "truncate" function produced different output than chalk generated natively.

Imho this is mostly a breaking change. Especially if some other library parses the output.

Much better than #251

I don't fully agree but the current maintainers can decide.

Personally I'm a bigger fan of smaller changes to resolve issues (less conflicts and issues with other projects, less breaking changes).

[jwalton]

Imho this is mostly a breaking change. Especially if some other library parses the output.

Just to clarify my comment; ANSI has escape sequences for setting the foreground color and the backgrond color. The tests I marked as "skip" were cases where instead of "reset foreground;reset background", we were now emitting "reset background;reset foreground". Any ANSI parser will treat these identically. Anyone who is parsing the output without a "real" ANSI parser will probably strip all the colors anyways.

I wouldn't consider it a breaking change. We are changing the output slightly, but any bug fix to any project changes something about its behavior.

[jwalton]

If we wanted to be execessively cautious we could pin the version and release a 0.6.1, and then merge this and release a 0.7.0, so we get the best of both worlds.

[DanielRuf]

If we wanted to be execessively cautious we could pin the version and release a 0.6.1

Already done, see #251 (comment)

[jwalton]

@Trott I raised Automattic/cli-table#166 for the original cli-table.

[DABH]

Please see #263 -- we can use the same colors interface and not be relying on a potentially compromised dependency. Hopefully this solution is the best of both worlds :)

[speedytwenty]

Thanks for your efforts on this, @jwalton! It looks like #263 supersedes this PR so closing this one.