D23718: WIP: Beginning of the Baloo/Search KCM Rewrite

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  See inline comments.
  
  Also functionality-wise there are some problems:
  
  1. When a new entry is added, if the KCM is closed and re-opened, the new entry has been forgotten
  2. When you add a path, the ugly URL schema bit doesn't get stripped off like it should: F7556828: Screenshot_20191009_090737.png <https://phabricator.kde.org/F7556828>

INLINE COMMENTS

> filteredfoldermodel.cpp:147
> +
> +    Q_FOREACH (const QString& mount, m_mountPoints) {
> +        if (url.startsWith(mount) && mount.size() > mountPoint.size())

Don't use Q_FOREACH in new or ported code

> main.qml:23
> +import QtQuick.Layouts 1.1
> +import QtQuick.Controls 2.12 as QQC2
> +import QtQuick.Dialogs 1.2 as QtDialogs

IIRC plasma isn't depending on Qt 5.12 yet, right?

> main.qml:39
> +        QQC2.Label {
> +            text: i18n("File Search helps you quickly locate all your files based on their content")
> +        }

This is a sentence, so it needs to end with a period.

> main.qml:47
> +            onCheckStateChanged: {
> +                if (!checkState === Qt.Unchecked) {
> +                    indexFileContents.checked = false;

This can be simplified to just `if (!checked)`

> main.qml:50
> +                }
> +                kcm.indexing = fileSearchEnabled.checked
> +            }

Don't need to reference it with `fileSearchEnabled`, just do `kcm.indexing = checked`

> main.qml:53
> +
> +            Component.onCompleted : {
> +                fileSearchEnabled.checked = kcm.indexing ? Qt.Checked : Qt.Unchecked

Don't do it this way, just use a binding:

`checked: kcm.indexing`

> main.qml:69
> +                indexFileContents.checked = kcm.fileContents ? Qt.Checked : Qt.Unchecked
> +            }
> +        }

Same comments as with the first checkbox

> main.qml:71
> +        }
> +
> +        QQC2.Label {

I would add some spacing between the last checkbox and the list view's explanatory label:

  Item {
      Layout.preferredHeight: Kirigami.Units.gridUnit
  }

> GB_2 wrote in main.qml:2
> Shouldn't this be you copyright?

Not done yet

> nicolasfella wrote in metadata.desktop:3
> That comment sounds wrong :)

Not done yet

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D23718

To: tcanabrava, #plasma, mart, ngraham
Cc: GB_2, nicolasfella, mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191009/2f1d4d86/attachment-0001.html>