AtomMaterialUI / color-highlighter

JetBrains plugin to preview colors directly from within the code.
MIT License
40 stars 23 forks source link

Short hex numbers in Scala sources should not be highlighted #123

Closed OndrejSpanel closed 10 months ago

OndrejSpanel commented 11 months ago

Originally described in now closed #101:

I find the current highlighting a bit too much aggressive. In some cases it highlights things which are not colors at all.

Short hex numbers in Scala sources:

image

    def hex(v: Int): ColorRGB = {
      val mask = 0xff
      fromBytes((v >> 16) & mask, (v >> 8) & mask, v & mask)
    }
mallowigi commented 10 months ago

I'm not a huge fan of adding a lot of settings. There's already quite a lot in my opinion.

But I'll leave this open, if more people are asking for this, I'll add more fine-tuning.

OndrejSpanel commented 10 months ago

Are people using 0xff or 0xff00 to write a color? I would use 0x0000ff or 0x00ff00 in such case.

mallowigi commented 10 months ago

good point...

mallowigi commented 10 months ago

The reason for that is that when it's a number, it actually checks which color is represented by that number. In numbers there's no such thing as length (in the same flavor as in strings). So in that case it would simply do the following math: 0xRRGGBB === (BB) + (GG) 256 + (RR) 256 * 256

So yeah 0xff = 255 == 0000FF therefore blue 0xff00 == f 16^3 + f 16^2 == 00FF00 therefore green etc.

Bottomline: in these cases there's no avoiding that except by converting the number to a hex representation and into a string, and in all honesty a bit too much work for my taste :p

But I encourage you to dirty your own hands and find a solution by yourself if you have time. OR implement the fine-tuned setting to disable detection in octal numbers.

OndrejSpanel commented 10 months ago

But I encourage you to dirty your own hands

If you agree we do not want to detect ff or ff00 as color, I will try if perhaps I am able to implement this.

mallowigi commented 10 months ago

no, we actually have to. Because 0x0000ff and 0xff are the same. And like i was saying, there's no "length" in the concept of numbers. The only way would be to create a helper that turns a number into a 6-digits hex representation and use the hex color parser to parse this.

OndrejSpanel commented 10 months ago

there's no "length" in the concept of numbers.

There is no length in numbers, but there is in their text representation. If I will create a PR which will highlight 0x0000ff but not 0xff or 0x00ff, is this something you would accept?

mallowigi commented 10 months ago

private fun parseHex(text: String): Color =

I suppose this is the part of the code that needs tweaking

OndrejSpanel commented 10 months ago

Thanks for your maintenance of the plugin. ❤️

If I have any more improvements, I will bother you again, but at the moment everything works fine for me.