Re: Review Request 119534: take defaults from a plasma look and feel package, if available
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 7, 2014, 3:19 p.m.) Review request for KDE Frameworks and Plasma. Changes --- tries a slightly different approach for the colors: if the lf package provides a file called colors, use that, otherwise read a colorscheme name from the defaults file. Opinions on what is the best way welcome Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs (updated) - CMakeLists.txt ef96e16 src/platformtheme/kdeplatformfiledialoghelper.h e2419e1 src/platformtheme/kdeplatformfiledialoghelper.cpp 799312e src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- src/platformtheme/khintssettings.cpp https://git.reviewboard.kde.org/r/119534/#comment7 you don't have to initialise SharedPtrs src/platformtheme/khintssettings.cpp https://git.reviewboard.kde.org/r/119534/#comment44451 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. src/platformtheme/khintssettings.cpp https://git.reviewboard.kde.org/r/119534/#comment9 so we don't want to be loading the cgIcons group then. src/platformtheme/khintssettings.cpp https://git.reviewboard.kde.org/r/119534/#comment44450 This means we'll end up with a KCM to change icons that does nothing. That sounds bad? I was under the impression that we were using LF to get the default icon set if one is not set. - David Edmundson On July 29, 2014, 4:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated July 29, 2014, 4:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp 104f77c Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 259 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line259 This means we'll end up with a KCM to change icons that does nothing. That sounds bad? I was under the impression that we were using LF to get the default icon set if one is not set. hmm, maybe there is some error in logic of readConfigValue.. but is supposed to first read from the user configured entry in kdeglobals, then if is empty try to read from lnf, and lastly if that's empty too read from the hardcoded parameter, so the user configured value should still win? - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On July 29, 2014, 4:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated July 29, 2014, 4:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp 104f77c Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 259 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line259 This means we'll end up with a KCM to change icons that does nothing. That sounds bad? I was under the impression that we were using LF to get the default icon set if one is not set. Marco Martin wrote: hmm, maybe there is some error in logic of readConfigValue.. but is supposed to first read from the user configured entry in kdeglobals, then if is empty try to read from lnf, and lastly if that's empty too read from the hardcoded parameter, so the user configured value should still win? Oh, I see it now. Sorry. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On July 29, 2014, 4:22 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated July 29, 2014, 4:22 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp 104f77c Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 53 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line53 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. ok, going to modify the name of the one provided in plasma-workspace then. - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs (updated) - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 53 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line53 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. Marco Martin wrote: ok, going to modify the name of the one provided in plasma-workspace then. Oh that might get quite invasive. I won't insist on doing that then. I guess other people would call it org.caledonia.lookandfeel, it's not like KDE would ever ship two (is it?) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 53 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line53 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. Marco Martin wrote: ok, going to modify the name of the one provided in plasma-workspace then. David Edmundson wrote: Oh that might get quite invasive. I won't insist on doing that then. I guess other people would call it org.caledonia.lookandfeel, it's not like KDE would ever ship two (is it?) Marco Martin wrote: as kde, probably going to be an org.kde.oxygen one, to have a quick kde4ish look so should this be org.kde.breeze.lookandfeel org.kde.oxygen.lookandfeel respectively - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 53 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line53 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. Marco Martin wrote: ok, going to modify the name of the one provided in plasma-workspace then. David Edmundson wrote: Oh that might get quite invasive. I won't insist on doing that then. I guess other people would call it org.caledonia.lookandfeel, it's not like KDE would ever ship two (is it?) Marco Martin wrote: as kde, probably going to be an org.kde.oxygen one, to have a quick kde4ish look David Edmundson wrote: so should this be org.kde.breeze.lookandfeel org.kde.oxygen.lookandfeel respectively bah, not necessarly, even just org.kde.breeze (or org.kde.loonandfeel for the default is fine as well) since they are installed under their own prefix (plasma/look-and-feel) they don't risk to clash with other types of packages anyways so lookandfeel in the name itself is not needed - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: src/platformtheme/khintssettings.cpp, line 53 https://git.reviewboard.kde.org/r/119534/diff/3/?file=294399#file294399line53 I don't think we want to call the default look and feel package something like maybe org.kde.defaultlookandfeel otherwise you're just putting the type in the name which is just confusing. Marco Martin wrote: ok, going to modify the name of the one provided in plasma-workspace then. David Edmundson wrote: Oh that might get quite invasive. I won't insist on doing that then. I guess other people would call it org.caledonia.lookandfeel, it's not like KDE would ever ship two (is it?) as kde, probably going to be an org.kde.oxygen one, to have a quick kde4ish look - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
On Aug. 4, 2014, 2:22 p.m., David Edmundson wrote: Another thing I would like an opinion on, is about the color scheme: right now it searches for a configured color scheme name, then searches in the package a file with the same name as the color scheme name. I was thinking it may be prettier to have the colors file optionally provided by the package as just colors or some fixed name? - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/#review63781 --- On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 3:18 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119534/#review63812
---
src/platformtheme/khintssettings.cpp
https://git.reviewboard.kde.org/r/119534/#comment44468
This is broken I think.
If I have my preferred style set to windows because the list already
contains that it won't change anything.
This means my list is still oxygen, fusion, windows in that order.
It think it should be
if (!lnfStyle.isEmpty()) {
styleNames.removeOne(lnfStyle);
styleNames.prepend(lnfStyle);
}
or we could change line 76 to use readConfigValue ?
- David Edmundson
On Aug. 4, 2014, 3:18 p.m., Marco Martin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119534/
---
(Updated Aug. 4, 2014, 3:18 p.m.)
Review request for KDE Frameworks and Plasma.
Repository: frameworkintegration
Description
---
there will be support for some kind of mega theme that besides providing
files for splashscreen, lockscreen etc, will define what defaults to use
among icons, colors, fonts etc.
This is a first start of it for reading the defaults. The theme file is
stored in a plasma package, but the patch resolves the correct path by hand,
not requiring linking to libplasma.
right now only a couple of settings is used, mostly to see if the direction
is good.
Diffs
-
src/platformtheme/khintssettings.h 57f1864
src/platformtheme/khintssettings.cpp f9e068d
Diff: https://git.reviewboard.kde.org/r/119534/diff/
Testing
---
Thanks,
Marco Martin
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated Aug. 4, 2014, 6:09 p.m.) Review request for KDE Frameworks and Plasma. Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs (updated) - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp f9e068d Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 119534: take defaults from a plasma look and feel package, if available
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119534/ --- (Updated July 29, 2014, 1:25 p.m.) Review request for KDE Frameworks and Plasma. Changes --- try to take the colorscheme name from the package Repository: frameworkintegration Description --- there will be support for some kind of mega theme that besides providing files for splashscreen, lockscreen etc, will define what defaults to use among icons, colors, fonts etc. This is a first start of it for reading the defaults. The theme file is stored in a plasma package, but the patch resolves the correct path by hand, not requiring linking to libplasma. right now only a couple of settings is used, mostly to see if the direction is good. Diffs (updated) - src/platformtheme/khintssettings.h 57f1864 src/platformtheme/khintssettings.cpp 104f77c Diff: https://git.reviewboard.kde.org/r/119534/diff/ Testing --- Thanks, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
