Look in XDG_DATA_DIRS for themes #210
Comments
|
@CDrummond The current paths and their priorities introduced several complications. A look at |
|
Are you sure? I was going to create a pull request this weekend - the change seemed trivial. e.g. Kvantum.cpp @761 Will you accept a pull request in this area? I'm happy to look into this. If you re-open, feel free to assign to me. |
Please don't do that! Contrary to its appearance, this thing is VERY complex. There are so many states that should be considered. It isn't maintainable -- in the real sense of the word and not according to what "didrocks" thinks about maintainability -- and isn't worth the effort. A long time ago, KDE devs said a similar thing about their Breeze/Oxygen style and I completely agree with them on that. That's while doing it with Breeze should be much "easier" than Kvantum ;) |
|
Please take a look at |
|
I know its complex. As stated, I'm willing to do the work. If you don't like the outcome, then you are free to reject any changes. Seeing as it is complex, surely making it simpler would be a good idea? Again, I'm volunteering to do this - with you having final approval. |
|
"Complex" is not a good description. It wouldn't be maintainable. Do you remember this: #152 ? It's just the tip of the iceberg. There should be a hierarchy of priorities when THIS_THEME is installed in multiple places; the absence of the SVG and/or kvconfig file should be taken into account and the nonexistent file should be taken from a theme with the same name but having a higher priority; Kvantum Manager should show the theme with the highest priority and be updated if it's deleted; dark-inside-light themes should be considered; and so on. It isn't like in GTK+. A Kvantum theme looks upward in the hierarchy of priorities and completes itself if possible. That's why what you want is practically impossible. Moreover and more importantly, if you write 100 lines of code (definitely, much more than that will be needed), I'll feel REALLY bad about rejecting it. Please consider that! Your time is valuable to me too. |
|
So, when the style starts up it needs to fine NAME.kvconfig and NAME.svg Therefore, the style needs to search within a pre-defined list of paths: e.g. for 'kvantum' style:
Search for kvconfig and svg file separately, returning the 1st of one of each found. Does it need to be more complex than this? |
|
If it was that "simple", I wouldn't write this -- and I intentionally paste it here to frighten you ;) /*******************
** kvconfig file **
*******************/
if (!userConfig.isEmpty())
{ // user theme
themeSettings_ = new ThemeConfig(userConfig);
}
else if (userSvg.isEmpty() // otherwise it's a user theme without config file
&& !themeName.endsWith("#")) // root theme names can't have the ending "#"
{ // root theme
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.kvconfig").arg(themeName);
if (QFile::exists(temp))
themeSettings_ = new ThemeConfig(temp);
else if (!isThemeDir(QString(DATADIR) + "/Kvantum", themeName) // svg shouldn't be found
&& isThemeDir(QString(DATADIR) + "/Kvantum", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.kvconfig").arg(lightName).arg(themeName);
if (QFile::exists(temp))
themeSettings_ = new ThemeConfig(temp);
}
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.svg").arg(themeName);
if (!QFile::exists(temp)) // otherwise the checked root theme was just an SVG image
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%1.kvconfig").arg(themeName);
if (QFile::exists(temp))
themeSettings_ = new ThemeConfig(temp);
}
if (!QFile::exists(temp)
&& !isThemeDir(QString(DATADIR) + "/themes", themeName)
&& isThemeDir(QString(DATADIR) + "/themes", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.svg").arg(lightName).arg(themeName);
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%2.kvconfig").arg(lightName).arg(themeName);
if (QFile::exists(temp))
themeSettings_ = new ThemeConfig(temp);
}
}
}
}
/***************
** SVG image **
***************/
if (!userSvg.isEmpty())
{ // user theme
themeRndr_ = new QSvgRenderer();
themeRndr_->load(userSvg);
}
else
{
if (!themeName.endsWith("#"))
{
if (userConfig.isEmpty()) // otherwise it's a user theme without SVG image
{ // root theme
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.svg").arg(themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
else if (!isThemeDir(QString(DATADIR) + "/Kvantum", themeName) // config shouldn't be found
&& isThemeDir(QString(DATADIR) + "/Kvantum", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.svg").arg(lightName).arg(themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.kvconfig").arg(themeName);
if (!QFile::exists(temp)) // otherwise the checked root theme was just a config file
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%1.svg").arg(themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
if (!QFile::exists(temp)
&& !isThemeDir(QString(DATADIR) + "/themes", themeName)
&& isThemeDir(QString(DATADIR) + "/themes", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.kvconfig").arg(lightName).arg(themeName);
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%2.svg").arg(lightName).arg(themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
}
}
}
}
else if (!userConfig.isEmpty()) // otherwise, the folder has been emptied manually
{ // find the SVG image of the root theme, of which this is a copy
QString _themeName = themeName.left(themeName.length() - 1);
if (!_themeName.isEmpty() && !_themeName.contains("#"))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.svg").arg(_themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
else if (isThemeDir(QString(DATADIR) + "/Kvantum", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.svg").arg(lightName).arg(_themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%1.kvconfig").arg(_themeName);
if (!QFile::exists(temp)) // otherwise the checked root theme was just a config file
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%1.svg").arg(_themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
if (!QFile::exists(temp)
&& !isThemeDir(QString(DATADIR) + "/themes", _themeName)
&& isThemeDir(QString(DATADIR) + "/themes", lightName))
{
temp = QString(DATADIR)
+ QString("/Kvantum/%1/%2.kvconfig").arg(lightName).arg(_themeName);
if (!QFile::exists(temp))
{
temp = QString(DATADIR)
+ QString("/themes/%1/Kvantum/%2.svg").arg(lightName).arg(_themeName);
if (QFile::exists(temp))
{
themeRndr_ = new QSvgRenderer();
themeRndr_->load(temp);
}
}
}
}
}
}
} |
|
Just because the current implementation (and I mean no insult here) is complex, does not mean it needs to be. What is wrong with the list I proposed? |
|
If it was all about a sketch, there would be no need to testing in the real world. For the "simple" implementation of dark-inside-light themes, several issues were surfaced later; it wasn't as simple as it looked. Yes, I can sacrifice borrowing files of another themes, checking theme directories for their sanity, etc. for covering more installation paths with a simpler code but the latter isn't worth the former, IMO. |
|
Obviously my list above is not catering for an issue you have seen, but I'm being a bit stupid and not seeing what it is. Would it not be better, and easier for theme authors? if there was a documented list (and order) of how the kvconfig and svg are located? I could then use that to adapt for XDG_DATA_DIRS, no? I guess I'm really missing something here. But 'all' the style needs to do is find a kvconfig and svg file. That should not be that complex. |
It's already documented. Theme makers use |
|
Then why all the other complexities? I can see your point about the current code being complex, and not wanting to needlessly make it more complex. However, adding XDG_DATA_DIRS which is cross distro standard seems like a very good idea to me. And I'm not asking you to make any changes, I will. All I need to know is why the list I proposed would not suffice, which corner cases it misses, and how to improve it. All I can see is a win-win. If I do it correctly (that's a big IF), then you get simpler code, and support standard directories. |
|
OK, do it. Feeling bad won't kill me ;) |
As well as looking in ${DATADIR}, ${XDG_DATA_DIRS} should also be inspected for themes. This can be accessed via
QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation)The text was updated successfully, but these errors were encountered: