Tests need somehow provide the current files and not rely on, that these
files are already installed (see D15727). Having no special
ordering in place, it is random, what file is picked, when there are
themes with same names on different locations. So we may need sort the
themesDirectories.
Details
Details
- Reviewers
mlaurent dfaure - Group Reviewers
KDE PIM - Commits
- R77:275785135605: Grantleetheme should always prefer files that are located in DataLocation.
Diff Detail
Diff Detail
- Repository
- R77 PIM: Grantlee Theme
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
The patch looks ok to me, but I'm not sure I understand the commit log, which talks about sorting. You don't need to sort, locateAll is sorted already (by order of the dirs in GenericDataLocation, which themselves have reliable order, e.g. it's the order of XDG_DATA_DIRS on linux)...
Comment Actions
from the patch itself it is not obvious, what is going wrong. I added a comment to the for loop (l127-l151) that is actually loading and overwriting the themes in a way you do not expect it in a Linux environment aka not able for the tests to set the themes (see D15727).
| src/grantleethememanager.cpp | ||
|---|---|---|
| 127 | @dfaure you see this for loop l127 - l151, this will actually overwrites a found theme with a theme found in an other directory. So in the end the most general directory wins. IMO that is the opposite is expected, so that the general folders are fallback, if the local one is not found. | |
Comment Actions
Pretty hard to reason about this from reading the code, but I'll try your debugging, and the removed code did look weird (count < 2 sounds very special-casey therefore broken).