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

Reply via email to