Re: [Development] Opinions on QTBUG-71545

2018-11-08 Thread Luca Beldi
I'm in favour of just adding a line of documentation and maybe include Thiago's 
workaround. Should take 30seconds and hurt nobody

Luca Beldi (VRonin)
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] QAbstractItemModel::setItemData behaviour

2018-09-03 Thread Luca Beldi
Gentle ping as I got no answers before.


Hi everyone,
While trying to submit a patch to fix QStringListModel::setItemData 
https://codereview.qt-project.org/235730 we opened a much larger discussion on 
QAbstractItemModel, that expanded on the forum 
https://forum.qt.io/topic/93207/qabstractitemmodel-setitemdata-partial-success  
but that I ultimately would like your opinion on:

At the moment, QAbstractItemModel::setItemData calls setData for each role 
until the first failure and returns false if that happens.
This opens up 2 issues:

* QAbstractItemModel::setItemData returns false even when some but not 
all data was actually set without rolling back the changes

* QAbstractItemModel::setItemData depends on the ordering of the roles

For the second point, an example that illustrates the problem is 
(QStringListModel::setItemData just uses the base class implementation):
QStringListModel::setItemData({{Qt::DisplayRole,QVariant("test")},{ 
Qt::DecorationRole,QVariant(QIcon())}) will return false but actually set the 
string to "test"
QStringListModel::setItemData({{Qt::EditRole,QVariant("test")},{ 
Qt::DecorationRole,QVariant(QIcon())}) will return false and do nothing on the 
model

My proposal is:

* Remove the ordering-dependency  QAbstractItemModel::setItemData (i.e. 
sets all the roles it can and returns false if any of them failed)

* Make setItemData in subclasses of QAbstractItemModel behave 
transaction-like (i.e. try to set all the roles, if any fails rollback the 
other changes and return false)

This however is technically a change of behaviour so I would like people way 
smarter than me to agree on the way forward.

Thanks,
Regards,
Luca Beldi (aka VRonin)

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


[Development] QAbstractItemModel::setItemData behaviour

2018-08-01 Thread Luca Beldi
Hi everyone,
While trying to submit a patch to fix QStringListModel::setItemData 
https://codereview.qt-project.org/235730 we opened a much larger discussion on 
QAbstractItemModel, that expanded on the forum 
https://forum.qt.io/topic/93207/qabstractitemmodel-setitemdata-partial-success  
but that I ultimately would like your opinion on:

At the moment, QAbstractItemModel::setItemData calls setData for each role 
until the first failure and returns false if that happens.
This opens up 2 issues:

* QAbstractItemModel::setItemData returns false even when some but not 
all data was actually set without rolling back the changes

* QAbstractItemModel::setItemData depends on the ordering of the roles

For the second point, an example that illustrates the problem is 
(QStringListModel::setItemData just uses the base class implementation):
QStringListModel::setItemData({{Qt::DisplayRole,QVariant("test")},{ 
Qt::DecorationRole,QVariant(QIcon())}) will return false but actually set the 
string to "test"
QStringListModel::setItemData({{Qt::EditRole,QVariant("test")},{ 
Qt::DecorationRole,QVariant(QIcon())}) will return false and do nothing on the 
model

My proposal is:

* Remove the ordering-dependency  QAbstractItemModel::setItemData (i.e. 
sets all the roles it can and returns false if any of them failed)

* Make setItemData in subclasses of QAbstractItemModel behave 
transaction-like (i.e. try to set all the roles, if any fails rollback the 
other changes and return false)

This however is technically a change of behaviour so I would like people way 
smarter than me to agree on the way forward.

Thanks,
Regards,
Luca Beldi (aka VRonin)

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development