[Development] How to fix TableView's API break?

[Richard Gustavsen]

Neither is ideal and I would've strongly preferred to stick to (row,column) everywhere. Everything else is an API trap waiting to happen.

Now that modelIndex() has been reverted to fix the SiC, I agree, we should consider deprecating/replacing the other two remaining functions that takes a column and a row as well, to make it consistent.

(Other ideas: remove the row/column convenience, and always use indices. Why does a _view_ return model indices at a given logical position, and not the model directly? Etc.)

In an TreeView, the application's tree model is flattened into an internal table model. So you cannot know, from the tree model alone, which QModelIndex a particular cell in the table represents. Hence, the view has to provide some mapping functions to help out (like e.g TableView.modelIndex(row, column)).

This is spelled "model index" in the entirety of Qt model/view code. Why can't the above be implemented like this?

let index = tableView.indexAtPosition(tapEvent.position) // this is a QModelIndex
tableView.positionViewAtIndex(index)

ListView was written to support models other than QAIM. And when TableView was implemented years later, we reused much of the same code (QQmlAdaptorModel, QQmlInstanceModel etc), to support the same. That said, adding more overloads to work directly with a QModelIndex, e.g positionViewAtIndex(), like you suggest, makes sense, and we should do that. But deprecating functions that takes a row, column (or a point), would mean that we would stop supporting those other models.

Wait, is this the reason -- that QModelIndex got "gadgetified" relatively recently (5.6?), and these views have been around since before it and couldn't use it?

ListView, even more so than TableView/TreeView, was written to support other models than just QAIM, where a QModelIndex cannot be used. I don’t know the history behind how/when/why QModelIndex became available to QML, though.

You could argue that we need a new type for this, and perhaps you’re right. But a table is, after all, just a two-dimensional grid of cells, and QPoint is meant as a general type for dealing with all kinds of two-dimensional coordinate systems.

That's very misleading.

For a cell position there's no sense of asking stuff like "what's the Manhattan distance from the origin" or "multiply its coordinates by a scalar factor", which are instead legitimate operations on a type like QPoint.

When implementing TableView, there would be no way around looking at a table as a two-dimensional grid of cells, where mathematical types like QPoint and QRect are used to implement rectangular selections, keep track of which cells are visible in the viewport, or simply navigating the current index around with the keyboard. Because of that (and the example I wrote earlier), I thought it would make sense to offer the same convenience to app developers. If that turns out to be a misguided attempt that has no valid use case (outside the internals of TableView), we can consider deprecating those (few) functions that takes a QPoint (and perhaps create a dedicated type, or drop support for models other than QAIM, and use QModelIndex).

Anyway, thanks for the input. I’m sure you’re not alone in thinking that the order of the arguments should have been swapped from the start. Feel free to create some JIRA tasks and assign them to me, so that we can keep track of suggestions and other things that should ideally be changed.

-Richard

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20221221/ebeacd45/attachment.htm>