D8215: Added support for STRINGS property in CMake cache editor

Kevin Funk noreply at phabricator.kde.org
Mon Oct 9 15:32:16 UTC 2017


kfunk added a comment.


  Do I understand correctly that all those columns are just here for storing data? They're not shown in the end.
  
  Then one should rather refactor this to set the data on each individual item via http://doc.qt.io/qt-5/qstandarditemmodel.html#setData instead of creating (hidden) columns. Even better: Turn this whole `CMakeCacheModel` into a proper model inheriting from `QAbstractItemModel` (more work). That way you're inherently required to implement this more cleanly :)
  
  Bonus points for introducing an enum for Name, Type, Value, ....
  
  @gracicot Not your fault of course, you just followed the existing pattern here.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8215

To: gracicot, #kdevelop, kfunk
Cc: apol, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171009/2241ff95/attachment.html>


More information about the KDevelop-devel mailing list