Fix #180 - Custom with multi attr failure

Marak / colors.js

get colors in your node.js console

https://github.com/Marak/colors.js

Other

5.17k stars 446 forks source link

Fix #180 - Custom with multi attr failure #232

Closed zucher closed 6 years ago

zucher commented 6 years ago

Fix #180 - Themes can have multiple attributes - but not in safe mode

DABH commented 6 years ago

Thanks for this PR! Can you explain your changes briefly to make reviewing easier? I'll try to take a look at this soon. Thanks!

Marak commented 6 years ago

Fixes generally should not be merged unless there is an accompanying test case which was previously failing before the PR fixed it.

zucher commented 6 years ago

The fix allows to manage correctly custom entry with multiple value, by returning an element colors[prop] with all specified colors., extraction of appProperty was done in order to mutualise both single and multiple entries (string and arrays).

DABH commented 6 years ago

Thanks, sounds good. As Marak says, it would be great if you could add a test so we know this is fixed and so we don't break this in the future. Then this should be ready to final review and merge if looks good.

zucher commented 6 years ago

Ok I'm currently working on ;-)

zucher commented 6 years ago

Done ;-)

DABH commented 6 years ago

Sorry for the delay -- just merged this. Thanks again for your contribution!