PR #575, which implements option 2, is now RFR.
-- Kevin
On 7/13/2021 7:58 AM, Kevin Rushforth wrote:
Here is a Draft PR that implements option 2:
https://github.com/openjdk/jfx/pull/575
It also fixes two additional issues with the properties:
* The docs for each property is on a private property field that does
have tree in the name, which results in no docs being generated for
either the tableRow or tableColumn property.
* The implementation of the tableRowProperty() method returns the
writable property by mistake rather than the read-only property that
is specified by the method's return type.
I will likely make it RFR tomorrow, and plan to target it to jfx17.
-- Kevin
On 7/12/2021 2:55 PM, Kevin Rushforth wrote:
In looking at this more closely, I now think that option 2 is the
better approach for the following reasons:
* TreeTablePosition extends TablePositionBase (which is also extended
by TablePosition), and inherits a getTableColumn() method that is
overridden with a covariant return type (using the type of the
generic "TC" parameter) of TreeTableColumn.
* Although not public API, TreeTableCellBehavior extends
TableCellBehaviorBase ( which is also extended by TableCellBehavior),
and inherits inherits a getTableColumn() method that is overridden
with a covariant return type of TreeTableColumn.
So option 2 is the one that will provide better consistency, in
addition to being less intrusive.
Thus, the only change I am proposing to the public API is:
getTreeTableRow() -> getTableRow() -- which I will do by deprecating
(not for removal) the existing method and adding getTableRow() to
match the property name.
-- Kevin
On 7/12/2021 10:22 AM, Kevin Rushforth wrote:
While evaluating a javadoc fix [1] that removes spurious warnings
for missing comments on JavaFX property methods, I looked at the
remaining warnings, and discovered an inconsistency in the naming of
two of the properties in the TreeTableCell class. I filed
JDK-8270314 [2] to track this.
Before I submit a PR, I wanted to solicit comments.
In TreeTableCell, there is a mismatch between name of the following
property method vs the getter:
tableRowProperty()
getTreeTableRow()
the get method has "Tree" in the name while the property method does
not.
The corresponding methods for column are self-consistent, and are
named without the "tree" in the name:
tableColumnProperty()
getTableColumn()
Given the type of the property -- TreeTableRow and TreeTableColumn,
respectively -- the properties should have "Tree" in the name, but
don't. Somewhat related is that the update methods for both row and
column are updateTreeTableColumn and updateTreeTableRow which do
have "TreeTable" in the name:
There are two possible solutions that seem workable:
Possible solutions:
1. Add Tree to all methods of the row and column properties
tableRowProperty() --> treeTableRowProperty()
tableColumnProperty() --> treeTableColumnProperty()
getTableColumn() --> getTreeTableColumn()
This is the most consistent, but is slightly more intrusive in that
changes 3 of the 4 public methods of the row and column properties.
2. Remove Tree from all methods of these properties
getTreeTableRow() -> getTableRow()
This is a less intrusive change that only affects one method. While
it makes the properties self-consistent and consistent with each
other, the names are not what would be expected given the return
type (TreeTableColumn and TreeTableRow), and are not consistent with
the update methods. Also, for applications that don't use the
property method, and only use the getter, it isn't any less change
than the the first option.
I propose to go with option #1 for maximum consistency and further
propose to deprecate *not for removal* the misnamed methods.
Comments?
-- Kevin
[1] https://github.com/openjdk/jdk/pull/4747
[2] https://bugs.openjdk.java.net/browse/JDK-8270314
