Mostly ColumnViewer changes, don't use ExpandableNode anywhere else

- reduced new API methods
- fixed javadoc
- renamed getVisibleLimitBasedChildren to getChildrenWithLimitApplied
- improved getChildrenWithLimitApplied implementation

Fixes eclipse-platform#818

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTableViewer.java

@@ -687,11 +687,9 @@ private void internalRefreshAll(boolean updateLabels) {
 		// e.g. if (a, b) is replaced by (b, a), the disassociate of b to
 		// item 1 could undo
 		// the associate of b to item 0.
-		Object[] children = null;
 		Item[] items = doGetItems();
-		if (getItemsLimit() > 0 && items.length > getItemsLimit()) {
-			children = getVisibleLimitBasedChildren(getRoot(), items);
-		} else {
+		Object[] children = getChildrenWithLimitApplied(getRoot(), items);
+		if (children == null) {
 			children = getSortedChildren(getRoot());
 		}
 		int min = Math.min(children.length, items.length);

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java

@@ -1995,15 +1995,12 @@ protected void internalRefresh(Widget widget, Object element,
 	 */
 	/* package */void internalRefreshStruct(Widget widget, Object element,
 			boolean updateLabels) {
-		Object[] updatedChildren = null;
 
 		// updateChildren will ask getSortedChildren for items to be populated.
 		// getSortedChildren always returns the limited items doesn't matter if there
-		// were any items were expanded. We need to fetch exactly same number of
+		// were any items expanded. We need to fetch exactly same number of
 		// elements which were shown in the viewer.
-		if (getItemsLimit() > 0) {
-			updatedChildren = getVisibleLimitBasedChildren(element, getChildren(widget));
-		}
+		Object[] updatedChildren = getChildrenWithLimitApplied(element, getChildren(widget));
 
 		updateChildren(widget, element, updatedChildren, updateLabels);
 		Item[] children = getChildren(widget);

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewer.java

@@ -18,7 +18,7 @@
 package org.eclipse.jface.viewers;
 
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -29,7 +29,6 @@
 import org.eclipse.jface.internal.InternalPolicy;
 import org.eclipse.jface.util.Policy;
 import org.eclipse.jface.viewers.internal.ExpandableNode;
-import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.DisposeEvent;
 import org.eclipse.swt.events.MouseAdapter;
 import org.eclipse.swt.events.MouseEvent;
@@ -849,18 +848,30 @@ protected Object[] getSortedChildren(Object parent) {
 	}
 
 	/**
-	 * {@link StructuredViewer#getSortedChildren(Object)} always returns the
-	 * complete list of elements to be populated. Now we have to apply viewer limit
-	 * for the items to be created on the viewer. Note that in case of parent is
-	 * {@link IExpandableNode} we will return next block of limited elements to be
-	 * created.
+	 * Apply items limit for the items to be created on the viewer. This method is
+	 * supposed to be called at the end of {@link #getSortedChildren(Object)} call,
+	 * which always returns the complete list of elements to be populated.
+	 * <p>
+	 * <ul>
+	 * <li>If the {@link #setItemsLimit(int)} is not used by viewer, this method
+	 * does nothing.</li>
+	 * <li>If the {@link #setItemsLimit(int)} is used by viewer, this method might
+	 * modify the list by reducing number of elements according to the limit set,
+	 * and by adding an {@link IExpandableNode} as last element.</li>
+	 * <li>If the {@link #getSortedChildren(Object)} is not overridden by the
+	 * subclass, this method shouldn't be used by clients.</li>
+	 * </ul>
+	 * Note that in case of parent is {@link IExpandableNode} we will return next
+	 * block of limited elements to be created.
 	 *
-	 * @param parent
-	 * @param sorted
+	 * @param parent parent element
+	 * @param sorted all children of given parent, as returned by
+	 *               {@link #getSortedChildren(Object)}
 	 * @return returns only limited items.
 	 * @since 3.31
+	 * @see ColumnViewer#setItemsLimit(int)
 	 */
-	protected Object[] applyItemsLimit(Object parent, Object[] sorted) {
+	protected final Object[] applyItemsLimit(Object parent, Object[] sorted) {
 		// limit the number of items to be created. sorted always gets the remaining
 		// elements to be created.
 		int itemsLimit = getItemsLimit();
@@ -894,10 +905,12 @@ protected Object[] applyItemsLimit(Object parent, Object[] sorted) {
 	}
 
 	/**
-	 * Viewer must dispose this cell and populate the remaining elements returned by
-	 * this cell.
+	 * Concrete viewer is supposed to "expand" {@link IExpandableNode} located at
+	 * given cell.
+	 * <p>
+	 * Default implementation does nothing.
 	 *
-	 * @param cell
+	 * @param cell selected on click
 	 * @since 3.31
 	 */
 	protected void handleExpandableNodeClicked(Widget cell) {
@@ -906,100 +919,104 @@ protected void handleExpandableNodeClicked(Widget cell) {
 	}
 
 	/**
-	 * We should not dispose already visible items and we should display limited
-	 * elements along with already visible items.
+	 * If the items limit is not set, or the number of visible children is below the
+	 * items limit, this method does nothing and returns {@code null}.
+	 * <p>
+	 * In other case, this method tries to fetch (probably updated) model elements
+	 * up to the number of given visible elements, expanding possible existing
+	 * {@link IExpandableNode} elements if needed.
+	 * <p>
+	 * Implementation note: we should not dispose already visible items and we
+	 * should display limited elements along with already visible items.
 	 *
 	 * @param parent          model item to be refreshed.
 	 * @param visibleChildren currently visible children. This includes any elements
 	 *                        already expanded by user.
-	 * @return list of children to be displayed/refreshed.
-	 * @since 3.31
+	 * @return list of children to be displayed/refreshed or {@code null} if given
+	 *         items length does not exceed items limit
 	 */
-	protected Object[] getVisibleLimitBasedChildren(Object parent, Item[] visibleChildren) {
-		Object[] updatedChildren = null;
+	Object[] getChildrenWithLimitApplied(Object parent, Item[] visibleChildren) {
 		final int limit = getItemsLimit();
-		// at least one iteration of expandable items are populated.
-		if (visibleChildren.length > limit) {
-			List<Object> listOfEle = new ArrayList<>(Arrays.asList(getSortedChildren(parent)));
-			// case 1 . if returned are less then limit just stop here and add list of ele
-			// to updatedChildren
-			if (listOfEle.size() < limit) {
-				updatedChildren = new Object[listOfEle.size()];
-				listOfEle.toArray(updatedChildren);
-			} else {
-				// numberOfTimesRead is equal to number of times it was expanded by user.
-				int numberOfTimesRead = visibleChildren.length / limit;
+		final int visibleItemsLength = visibleChildren.length;
+		if (visibleItemsLength <= limit || limit <= 0) {
+			return null;
+		}
+		// if we are here, at least one iteration of expandable items is shown
 
-				if (visibleChildren.length % limit > 0
-						&& !(visibleChildren[visibleChildren.length - 1].getData() instanceof IExpandableNode)) {
-					numberOfTimesRead++;
-				}
-				List<Object> newList = new ArrayList<>();
-				Object ele1 = parent;
-
-				while (numberOfTimesRead > 0) {
-					List<Object> subList = new ArrayList<>(Arrays.asList(getSortedChildren(ele1)));
-					// no more to read. case 2. possibility of less items than visibleChildren but
-					// more than basic limit.
-					if (!(subList.get(subList.size() - 1) instanceof IExpandableNode)) {
-						newList.addAll(subList);
-						break;
-					}
-					numberOfTimesRead--;
-					// decide to remove last element or not
-					if (numberOfTimesRead > 0) {
-						ele1 = subList.remove(subList.size() - 1);
-					}
-					newList.addAll(subList);
-				}
-				updatedChildren = new Object[newList.size()];
-				newList.toArray(updatedChildren);
+		Object[] currentSorted = getSortedChildren(parent);
+		// if current children count is below limit just return them
+		if (currentSorted.length < limit) {
+			return currentSorted;
+		}
 
+		// numberOfTimesRead is equal to number of times it was expanded by user.
+		int numberOfTimesRead = visibleItemsLength / limit;
+		if ((visibleItemsLength % limit) > 0
+				&& !(visibleChildren[visibleItemsLength - 1].getData() instanceof IExpandableNode)) {
+			numberOfTimesRead++;
+		}
+		List<Object> newChildren = new ArrayList<>();
+		Object lastElement = parent;
+		while (numberOfTimesRead > 0) {
+			Object[] sortedChildren;
+			if (lastElement == parent) {
+				sortedChildren = currentSorted;
+			} else {
+				sortedChildren = getSortedChildren(lastElement);
+			}
+			lastElement = sortedChildren[sortedChildren.length - 1];
+			Collections.addAll(newChildren, sortedChildren);
+
+			if (!(lastElement instanceof IExpandableNode)) {
+				// no more to read: less items than visibleChildren but more than limit.
+				break;
+			}
+			numberOfTimesRead--;
+			if (numberOfTimesRead > 0) {
+				// remove expandable node from the end of the list
+				// we will expand it at least once more on next loop round
+				newChildren.remove(newChildren.size() - 1);
 			}
 		}
-		return updatedChildren;
+		return newChildren.toArray(new Object[newChildren.size()]);
 	}
 
 	/**
-	 * Returns the current viewer limit.
+	 * Returns the current viewer limit on direct children at one level. Limit that
+	 * is less than or equal to zero has no effect on the viewer.
 	 *
-	 * @return limit
+	 * @return current limit
 	 * @since 3.31
 	 */
 	public int getItemsLimit() {
 		return itemsLimit;
 	}
 
 	/**
-	 * Sets the viewers items limit. This must be called before
-	 * {@link Viewer#setInput(Object)}. A parameter less than or equal to 0 OR
-	 * greater than length of input has no effect on the viewer.
+	 * Sets the viewers items limit on direct children at one level.
+	 * <p>
+	 * If the number of direct children will exceed this limit, the viewer will only
+	 * show a subset of children up to the limit and add an {@link IExpandableNode}
+	 * element after last shown item.
+	 * </p>
 	 * <p>
-	 * This will limit the number of items to be rendered on any level. It creates
-	 * an {@link IExpandableNode} at the end of limited items which will then
-	 * creates next block of items on receiving {@link SWT#MouseDown} event.
+	 * This method must be called before {@link #setInput(Object)}. A parameter less
+	 * than or equal to zero has no effect on the viewer.
 	 * </p>
 	 * <p>
-	 * Client can adapt {@link ILabelProvider} to provide desired label and image
-	 * for {@link IExpandableNode} otherwise default label and no image will be
+	 * Client can adapt {@link ILabelProvider} to provide custom label and icon for
+	 * {@link IExpandableNode} element, otherwise default label and no icon will be
 	 * used.
 	 * </p>
 	 *
-	 * @param newLimit A non-negative integer greater than 0.
+	 * @param newLimit A non-negative integer greater than 0 to enable items limit
 	 * @since 3.31
 	 */
 	public void setItemsLimit(int newLimit) {
 		itemsLimit = newLimit;
 	}
 
-	/**
-	 * @param result
-	 * @param startOffSet
-	 * @param limit
-	 * @return new node
-	 * @since 3.31
-	 */
-	protected IExpandableNode createExpandableNode(Object[] result, int startOffSet, int limit) {
+	IExpandableNode createExpandableNode(Object[] result, int startOffSet, int limit) {
 		ExpandableNode expandableNode = new ExpandableNode(result, startOffSet, limit);
 		expandableNodes.add(expandableNode);
 		return expandableNode;
@@ -1014,11 +1031,7 @@ protected void disassociate(Item item) {
 		super.disassociate(item);
 	}
 
-	/**
-	 * @return Returns the expandableNodes.
-	 * @since 3.31
-	 */
-	protected Set<IExpandableNode> getExpandableNodes() {
+	Set<IExpandableNode> getExpandableNodes() {
 		return expandableNodes;
 	}
 

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TableViewer.java

@@ -20,7 +20,6 @@
 package org.eclipse.jface.viewers;
 
 import org.eclipse.core.runtime.Assert;
-import org.eclipse.jface.viewers.internal.ExpandableNode;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.widgets.Composite;
@@ -457,9 +456,6 @@ protected Widget doFindItem(Object element) {
 		return super.doFindItem(element);
 	}
 
-	/**
-	 * @since 3.30
-	 */
 	@Override
 	protected void handleExpandableNodeClicked(Widget item) {
 		if (!(item instanceof Item)) {
@@ -484,7 +480,7 @@ protected void handleExpandableNodeClicked(Widget item) {
 			// If we've expanded but still have not reached the limit
 			// select new expandable node, so user can click through
 			// to the end
-			if (getLastElement() instanceof ExpandableNode node) {
+			if (getLastElement() instanceof IExpandableNode node) {
 				setSelection(new StructuredSelection(node), true);
 			} else {
 				// set selection to first element of newly created items.

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java

@@ -25,7 +25,6 @@
 import java.util.List;
 
 import org.eclipse.jface.util.Policy;
-import org.eclipse.jface.viewers.internal.ExpandableNode;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.TreeEvent;
 import org.eclipse.swt.events.TreeListener;
@@ -1132,9 +1131,6 @@ public void editElement(Object element, int column) {
 		}
 	}
 
-	/**
-	 * @since 3.30
-	 */
 	@Override
 	protected void handleExpandableNodeClicked(Widget item) {
 		if (item == null) {
@@ -1171,7 +1167,7 @@ protected void handleExpandableNodeClicked(Widget item) {
 			// select new expandable node, so user can click through
 			// to the end
 			Object lastElement = getLastElement(parent);
-			if (lastElement instanceof ExpandableNode node) {
+			if (lastElement instanceof IExpandableNode node) {
 				setSelection(new StructuredSelection(node), true);
 			} else {
 				// set selection to first element of newly created items.