RFR: 6810: Create treemap viewer for JOverflow [v3]

[Kangcheng Xu]

On Mon, 15 Jun 2020 18:47:45 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

>> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed failing to create the part's controls due to NPE
>
> application/org.openjdk.jmc.joverflow.ext.treemap/src/main/java/org/openjdk/jmc/joverflow/ext/treemap/swt/Breadcrumb.java
> line 350:
>> 349: 	 */
>> 350: 	public BreadcrumbItem getSelection() {
>> 351: 		checkWidget();
> 
> does this get called anywhere?

No, it doesn't, but it makes sense to have this public method to an SWT viewer, just like `Tree` and `List`. I believe
this allows potential new features in the future to work together with a `Breadcrumb`.

> application/org.openjdk.jmc.joverflow.ext.treemap/src/main/java/org/openjdk/jmc/joverflow/ext/treemap/swt/TreemapToolTip.java
> line 55:
>> 54: 	protected Composite createToolTipContentArea(Event event, Composite parent) {
>> 55: 		Composite ret = new Composite(parent, SWT.NONE);
>> 56:
> 
> minor nit regarding variable name choice, is ret supposed to be rect?

It actually means "return" (value). Bad naming here. I've changed this to `container` instead.

> application/org.openjdk.jmc.joverflow.ext.treemap/src/main/java/org/openjdk/jmc/joverflow/ext/treemap/TreemapPage.java
> line 70:
>> 69: /* package-private */ class TreemapPage extends Page implements ModelListener {
>> 70: 	private static final Color[] COLORS = {new Color(Display.getCurrent(), 250, 206, 210), // red
>> 71: 			new Color(Display.getCurrent(), 185, 214, 255), // blue
> 
> Note: I thought this line looked a bit "off" with the color directly proceeding the `{`, but this seems to be the way
> spotless wants it to be formatted. Just a heads up to anyone else checking this out...

I've added a `//` to make spotless happy with starting the array content on a new line.

> application/org.openjdk.jmc.joverflow.ext.treemap/src/main/java/org/openjdk/jmc/joverflow/ext/treemap/TreemapAction.java
> line 41:
>> 40:
>> 41: /* package-private */ class TreemapAction extends Action {
>> 42: 	private final TreemapActionType actionType;
> 
> I guess this comment kind of applies across the whole patch; I'm not against having a package private comment (it's
> kind of nice for the methods at least IMO), however if kept I'd prefer if they followed a similar standard.
> For example, here it's placed before the class, however elsewhere like `Treemap.checkNull()` it's on the previous line.

`/* package-private */` comments are now consistently placed on the same lines as the function/class it's referring to.

-------------

PR: https://git.openjdk.java.net/jmc/pull/77