D14774: Add QML Extensions API
David Rosca
noreply at phabricator.kde.org
Fri Sep 21 08:48:28 BST 2018
drosca requested changes to this revision.
drosca added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> config.h.cmake:13
> +
> +#cmakedefine HAVE_LIBINTL
It should be `#cmakedefine01 HAVE_LIBINTL`.
> history.cpp:330
> + if (query.next()) {
> + entry = new HistoryEntry;
> + entry->count = query.value(0).toInt();
Same issue here, nothing will delete it.
> drosca wrote in qmlextensionscheme.cpp:31
> So this was discussed in the previous review and the outcome was that it will not work like that, and it must pass QJSValue in the signal. Am I correct?
It must be `requestStarted(const QJSValue &request)`.
> qmlhistory.cpp:44
> + list.reserve(result.size());
> + for (HistoryEntry entry : result) {
> + auto item = QmlStaticData::instance().getHistoryItem(entry);
`const HistoryEntry &entry`
> drosca wrote in qmlplugininterface.cpp:121
> Use `QQuickWidget` instead.
This should return `nullptr`. Again did you test it?
You should use `QQuickWidget` instead of `QWidget::createWindowContainer`.
> qmlstaticdata.cpp:83
> +{
> + QmlHistoryItem *item = nullptr;//m_historyItems.value(entry);
> + if (!item) {
?
> qmlstaticdata.h:84
> +
> +inline bool operator ==(HistoryEntry x, HistoryEntry y) {
> + return x.title == y.title && x.urlString == y.urlString && x.date == y.date && x.count == y.count;
const-ref
> qmlstaticdata.h:89
> +inline uint qHash(HistoryEntry entry) {
> + return qHash(QString("%1 %2 %3 %4").arg(entry.title, entry.urlString, entry.date.toString(), QString::number(entry.count)));
> +}
QSL
> metadata.desktop:6
> Name[cs]=Načtení prostředním tlačítkem myši
> +Name[en_GB]=MiddleClickLoader
> Name[es]=Cargador de clic central
These translations changes are unrelated and should not be here.
REPOSITORY
R875 Falkon
REVISION DETAIL
https://phabricator.kde.org/D14774
To: anmolgautam, drosca
Cc: falkon, herrold, anmolgautam, SGOrava, iodelay, spoorun, ptabis, navarromorales, cochise, clivej, ach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/falkon/attachments/20180921/9bbe18db/attachment.html>
More information about the Falkon
mailing list