D25604: Properly add data root to XDG_DATA_DIRS and only when necessary

[Aleix Pol Gonzalez]

apol requested changes to this revision.
apol added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> startplasma.cpp:188
>  
> -    // Make sure that the KDE prefix is first in XDG_DATA_DIRS and that it's set at all.
> -    // The spec allows XDG_DATA_DIRS to be not set, but X session startup scripts tend
> -    // to set it to a list of paths *not* including the KDE prefix if it's not /usr or
> -    // /usr/local.
> -    if (!qEnvironmentVariableIsSet("XDG_DATA_DIRS")) {
> -        qputenv("XDG_DATA_DIRS", KDE_INSTALL_FULL_DATAROOTDIR ":/usr/share:/usr/local/share");
> +    // If the kde installation is in a non-standard location (ie. with a dev build),
> +    // it is possible the data root is not in the XDG_DATA_DIRS variable yet.

Please reduce the amount of comments.
It's hard to read and not very descriptive. We can read the code as well. Instead of writing what it does, explain why if you really think it's necessary. At this point I'd be happy removing the comments overall.

> startplasma.cpp:208
> +    QString kdeInstallFullDataRootDir = QStringLiteral(KDE_INSTALL_FULL_DATAROOTDIR);
> +    QStringList xdgDataDirs = xdgDataDirsValue.split(QLatin1Char(':'));
> +    if (!xdgDataDirs.contains(kdeInstallFullDataRootDir)) {

Can do with splitRef.

REPOSITORY
  R120 Plasma Workspace

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

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