search

SlexAxton / css-colorguard

Keep a watchful eye on your css colors.
2.44k stars 65 forks source link

How to handle the same colors represented differently? #40

Closed davidtheclark closed 8 years ago

davidtheclark commented 8 years ago

I ran into a situation where the same colors was represented in short and long hex format, #888 and #88888, and css-colorguard complained. Since those are the same color, not with a slight but hardly distinguishable difference, I thought may it wasn't this plugin's place to complain about it. So I wrote this test and added some logic to handle it.

But that made a couple of existing tests fail. The existing tests are relying on the fact that the same color represented two ways (in these tests, hex and rgba()) should fail. I don't see this documented, though (am I missing that)?

So I opened this to ask about the thinking here and see if there's a deliberate choice to complain about equal colors. If so why, and can we document it? Thanks!

SlexAxton commented 8 years ago

I honestly don't know if I have an answer to this.

On one hand, it seems silly that we'd warn you for something like using 3-char vs 6-char notation for the same color.

On the other hand, this is a "linter" style program with the goal of making your code better. I think consistent notation is a good property of consistent code.

I think it comes down to context. Smart developers who have a good grasp on what's going on should be trusted that they won't abuse the freedom of notation, and that they'll use rgba when it's appropriate and sometimes it might end up white.

Soooooo...

It feels like an option!

--allow-equivalent-notation or something.

I feel like the current defaults are correct but am super-open to discussion. I think it'd be great to allow folks to opt-in to using any equivalent notation. Does that sound ok to you? Are you interested in writing that patch?

Thanks! Alex

ben-eb commented 8 years ago

I would agree that consistent notation is a good thing in a CSS codebase, so I would also prefer this as an off by default option.

davidtheclark commented 8 years ago

Sure, option seems good to me. I think I could supplement this PR with that option and add some docs.

davidtheclark commented 8 years ago

I added the option and tried to document it everywhere.

davidtheclark commented 8 years ago

Any problems with the current code?