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