D7164: KSqueezedTextLabel: Respect indent, margin and frame width

Dominik Haumann noreply at phabricator.kde.org
Sun Aug 6 21:31:36 UTC 2017


dhaumann added a comment.


  This patch adds four functions that all "reimplement" functions that exist in base classes.
  However, these functions are not virtual. As such, adding the functions is probably fine (BC), but essentially the correct behavior now depends on calling the correct function (namely the ones from KSqueezedTextLabel).
  
  This by itself is a bit unfortunate, since it opens doors for bugs. This, however, is not properly fixable for KF 5 / Qt 5, so I guess this approach is fine.
  
  The functions itself are ok, but I think the remark "Reimplementation of... but see setText() for additional remark" is a bit fuzzy. I would prefer to copy the "@warning This function in the base class is not virtual. Therefore make sure to call this function" or some similar text. This would be much more explicit. And again, I would prefer having such a remark also in the detailed description of the class itself.
  
  Any other comments would be very much appreciated :-)
  
  Finally, would that be fixable for Qt 6? Maybe instead of deriving from QLabel, derive from QWidget and then *use* a QLabel internally as member variable? Nothing for this patch, but still maybe worth to think about. If we have a proper solution, we should add a comment like // TODO: KF6 ...

REPOSITORY
  R236 KWidgetsAddons

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

To: rkflx, #frameworks
Cc: dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170806/39f432fa/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list