ngraham added a comment.
Ok, will do. I just have a few comments about the backend bits, just minor
stuff. Overall this is really good! I plan to polish the UI after this lands,
but left comments on the front-end component anyway so you can sharpen your QML
skills (which are quite good already). :) Feel free to consider them more
educational than actionable. :)
INLINE COMMENTS
> filteredfoldermodel.cpp:36
> namespace {
> + QString normalizeTrailingSlashes(QString&& input) {
> + if (!input.endsWith('/'))
this handy little function feels like it wants to be in KCoreAddons or something
> filteredfoldermodel.cpp:47
> + if (!str.endsWith('/'))
> + str += QLatin1Char('/');
> }
Could probably just call `normalizeTrailingSlashes()` here
> filteredfoldermodel.cpp:83
> + if (displayName.startsWith(homePath))
> + displayName.replace(0, homePath.length(), QStringLiteral("~/"));
>
use braces for single-line if statements here, as done immediately below
> filteredfoldermodel.cpp:208
> + auto excluded = addTrailingSlashes(m_settings->excludedFolders());
> + auto included = addTrailingSlashes(m_settings->folders());
> + if (entry.enableIndex) {
There's a lot of use of `auto` in this file for simple `QStringList` objects
where I think it would be better to just declare the type
> main.qml:100
>
> RowLayout {
> + Layout.alignment: Qt.AlignRight
Since this layout now only has one item in it, there's no longer a need for a
layout at all
> main.qml:112
> +
> + Component {
> + id: directoryConfigDelegate
Since this is only used once, it could simply be declared inline
delegate: Kirigami.SwipeListItem {
id: listItem
onClicked: {
[blabla]
}
> main.qml:122
> +
> + RowLayout {
> + Kirigami.Icon {
set spacing: units.smallSpacing on the layout instead of adding an empty item
between objects for spacing
> main.qml:133
> + ColumnLayout {
> + RowLayout {
> + Kirigami.Icon {
ditto
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D28025
To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
