| ndavis | |
| davidedmundson |
| Plasma | |
| VDG |
BUG: 415461
FIXED-IN: 5.18.0
Depends on D26510
Right now when you first change the wallpaper plugin to slideshow, no locations are
pre-configured, so you have to add one yourself. This patch adds a
preferred:wallpaperlocations token as the default that gets replaced in the user's
config file at runtime with the XDG wallpaper locations.
Log in as new user > right-click desktop > configure > change to slideshow wallpaper
See that`/usr/share/wallpapers` is already in the list of wallpaper locations; how nice
| No Linters Available |
| No Unit Test Coverage |
| Buildable 20804 | |
| Build 20822: arc lint + arc unit |
Wondering if this is something that should be set in CMake or if it's so universal that it's okay to have it hardcoded in the file like this?
Perhaps we could leverage some code from Task Manager to have a fake preferred://wallpaperlocation URL or something like that?
Couldn't we just use wrap the folders from the single image case in a code tag?
<code>QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("wallpapers/"), QStandardPaths::LocateDirectory)</code>
I don't think so, as we use a runtime evaluation of kcfg files within plasma. Code tags won't work there.
That might be able to work. @davidedmundson, could this be a path forward, or did you have something else in mind?
| wallpapers/image/image.cpp | ||
|---|---|---|
| 360–367 | I know this was there before but didn't work before as intended because of the wrong path but imo now that there's a default in kcfg if the path is empty it should stay empty and not magically go back to the default if the user removes every path. | |
| wallpapers/image/image.cpp | ||
|---|---|---|
| 362 | there's two mistakes that cancel kinda each other out here. typically, this should be qAsConst(m_slidePaths) https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops/ BUT: you modify m_slidePaths whilst you are iterating through m_slidePaths. If we hadn't accidentally detached this would be very crashy dangerous code. Feel free to claim it was deliberate because you are a genius. However, rather than a deep copy ideally we would just force a shallow copy i.e const QStringList preprocssedPaths = m_slidePaths; best of all worlds | |
| wallpapers/image/image.cpp | ||
|---|---|---|
| 362 | haha, in fact it was deliberate, but I'm hardly a genius, and your suggestion makes more sense. I'll do that. | |