broulik added inline comments.
INLINE COMMENTS
> FormLayoutGallery.qml:79
> + text: item ? "Remove Field" : "Add Field"
> + property var item
> + onClicked: {
`property Item`?
That's in a couple of places
> FormLayout.qml:45
> +
> + Timer {
> + id: hintCompression
Use `Qt.callLater` (new in Qt 5.8) if you pass it an actual function (ie. not
an inline one) it will compress subsequent calls to one:
https://doc.qt.io/qt-5/qml-qtqml-qt.html#callLater-method
There's probably more places in the code where you could do this.
> formlayoutattached.cpp:28
> + m_buddyFor = qobject_cast<QQuickItem *>(parent);
> + qApp->installEventFilter(this);
> +}
This class does not have an `eventFilter`
> formlayoutattached.h:37
> +
> + explicit FormLayoutAttached(QObject *parent = 0);
> + ~FormLayoutAttached();
`nullptr`
> formlayoutattached.h:38
> + explicit FormLayoutAttached(QObject *parent = 0);
> + ~FormLayoutAttached();
> +
`override`
> formlayoutattached.h:56
> + void isSectionChanged();
> + void mnemonicChanged();
> + void decoratedLabelChanged();
Unused, there's also no property.
> formlayoutattached.h:57
> + void mnemonicChanged();
> + void decoratedLabelChanged();
> +
Also unused
> formlayoutattached.h:61
> + QString m_label;
> + QString m_actualDecoratedLabel;
> + QString m_decoratedLabel;
Unused
> formlayoutattached.h:62
> + QString m_actualDecoratedLabel;
> + QString m_decoratedLabel;
> + QPointer <QQuickItem> m_buddyFor;
Unused
> mnemonicattached.cpp:52
> +{
> + if (s_objectToSequence.contains(this)) {
> + s_sequenceToObject.remove(s_objectToSequence.value(this));
Double lookup (contains+value), also below
> mnemonicattached.cpp:62
> +
> + if (m_richTextLabel.length() == 0) {
> + return false;
`isEmpty`
> mnemonicattached.cpp:167
> + m_mnemonicLabel = m_actualRichTextLabel;
> + emit mnemonicLabelChanged();
> + emit richTextLabelChanged();
Can we emit this stuff only if actually changed or can this be assumed here?
> mnemonicattached.cpp:229
> +{
> + return m_actualRichTextLabel.length() > 0 ? m_actualRichTextLabel :
> m_label;
> +}
`!isEmpty?
> mnemonicattached.cpp:287
> + } else {
> + m_weight = m_baseWeight + m_weights.keys().last();
> + }
`(m_weights.constEnd() - 1).key()`? Avoids creating a `keys()` temporary list
> mnemonicattached.h:38
> + Q_PROPERTY(QKeySequence sequence READ sequence NOTIFY sequenceChanged)
> + Q_ENUMS(ControlType)
> +public:
Use `Q_ENUM`
> mnemonicattached.h:48
> +
> + explicit MnemonicAttached(QObject *parent = 0);
> + ~MnemonicAttached();
`nullptr`
REPOSITORY
R169 Kirigami
REVISION DETAIL
https://phabricator.kde.org/D8641
To: mart, #plasma, #kirigami, hein
Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ZrenBot,
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart,
hein
