Bug 360874

Comment 1 Nick Sandonato 2011-10-13 14:10:59 EDT

This is indeed something I always thought could be handy.

Comment 2 Nick Sandonato 2011-11-28 17:08:15 EST

Created attachment 207631 [details]
patch

Comment 3 Nick Sandonato 2011-11-28 17:39:05 EST

Released the changes to HEAD for Juno.

Comment 4 Nick Sandonato 2011-11-28 17:41:33 EST

Oh, and it's implemented as a text hover. So just hover over a color value and it'll popup with a square with the appropriate color.

Comment 5 Victor Homyakov 2011-11-29 06:06:25 EST

(In reply to comment #3)
> Released the changes to HEAD for Juno.

Will it work if I install it in Eclipse 3.7.1? I would like to use it ASAP :)

Comment 6 Victor Homyakov 2011-11-29 06:17:17 EST

(In reply to comment #2)
> Created attachment 207631 [details]

Check for null at line 52 is redundant:

52	if (region != null) {
53		if (region instanceof ICSSPrimitiveValue) {
54			return getColorValue((ICSSPrimitiveValue) region);
55		}
56	}

- in this case instanceof will return false accordingly to JLS, so code may be simplified to

	if (region instanceof ICSSPrimitiveValue) {
		return getColorValue((ICSSPrimitiveValue) region);
	}

Comment 7 Victor Homyakov 2011-11-29 08:19:14 EST

Also the code in private RGB getRGBFromHex(String hex) can be simplified (no loops, arrays, StringBuffers, string copying):

private RGB getRGBFromHex(String hex) {
	int r, g, b;
	try {
		if (hex.length() == 4) { // 3-digit notation "#rgb"
			r = Integer.parseInt(hex.substring(1, 2), 16);
			g = Integer.parseInt(hex.substring(2, 3), 16);
			b = Integer.parseInt(hex.substring(3, 4), 16);
			r = (r << 4) | r;
			g = (g << 4) | g;
			b = (b << 4) | b;
			return new RGB(r, g, b);
		} else if (hex.length() == 7) { // 6-digit notation "#rrggbb"
			r = Integer.parseInt(hex.substring(1, 3), 16);
			g = Integer.parseInt(hex.substring(3, 5), 16);
			b = Integer.parseInt(hex.substring(5, 7), 16);
			return new RGB(r, g, b);
		}
	} catch (NumberFormatException e) { // Invalid hexcode used.
	}
	return null;
}

Comment 8 Nick Sandonato 2011-11-29 10:30:55 EST

(In reply to comment #5)
> (In reply to comment #3)
> > Released the changes to HEAD for Juno.
> 
> Will it work if I install it in Eclipse 3.7.1? I would like to use it ASAP :)

Nope. The new functionality will be in Juno.

Thanks for checking out the code. I removed the redundant null check between the time I attached this patch and releasing the code. I'll simplify the hex extraction, not quite sure why I chose the loop route.