We create overload signal but we can use it with argument direcly
Details
build
Diff Detail
- Repository
- R236 KWidgetsAddons
- Branch
- remove_overload_signal (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 19239 Build 19257: arc lint + arc unit
src/kurllabel.h | ||
---|---|---|
284 | Wrong comment? |
Looks correct from deprecation-markup POV. Did you do a test build with EXCLUDE_DEPRECATED_BEFORE_AND_AT=5.65.0, to check if any tests or examples still use the deprecated signals and need some additions of #if KWIDGETSADDONS_ENABLE_DEPRECATED_SINCE(5, 65) in places?
When it comes to deprecated signals, seems those overloads have been present since introduction of KUrlLabel. Not sure about the usefulness of passing the url as argument with any signals, given that the url is a static property of the label, only set via the setter setUrl().
Have you checked how the respective signals are used? Are there similar classes, what signals do they have?
There is not autotests/test which use kurllabel.
we have some apps as connect(urlLabel, qOverload<const QString &>(&KUrlLabel::leftClickedUrl), this, &SummaryWidget::selectFolder); in kmail.
Yeah, but these days with lambdas, we can find out who emitted the signal, without the need for an identifier as signal argument...
connect(urlLabel, qOverload<>(&KUrlLabel::leftClickedUrl), this, [this, urlLabel]() { selectFolder(urlLabel->url()); } );
should work too.
This way the application can choose whether it wants the URL or the KUrlLabel pointer in the slot, or whatever else.
I'm not 100% sure what's best -- e.g. QPushButton has clicked() without arguments, while KJob as result(KJob*) because it's convenient for the slot. With lambdas we don't really need the KJob* anymore. Capturing the local job variable instead would even remove some static_cast to subclasses in a number of cases. But the porting effort would be huge, so I'm reluctant :-)
I guess this is most closely related to QPushButton though. In the simple case (different slot for each button or URLLabel) the signal with 0 arguments was always enough, and for the dynamic case (N buttons or N URLLabels) we have lambdas nowadays. Seems true for both, so I vote for deprecating the signals with an argument, and for porting KMail to my lambda above right away.
Opinions?
I don't prefer a signal with argument or not.
Indeed I can adapt kmail for it.
So I will change deprecated signal.