ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> fonts.cpp:131
> + for (int t = KXftConfig::SubPixel::None; t <=
> KXftConfig::SubPixel::Vbgr; ++t) {
> + QStandardItem *item = new
> QStandardItem(KXftConfig::description((KXftConfig::SubPixel::Type)t));
> + m_subPixelOptionsModel->appendRow(item);
Please no C cast, use static_cast instead.
Also was a good place to use auto (DRY and all that) ;-)
> fonts.cpp:136
> + for (int s = KXftConfig::Hint::None; s <= KXftConfig::Hint::Full; ++s) {
> + QStandardItem * item = new
> QStandardItem(KXftConfig::description((KXftConfig::Hint::Style)s));
> + m_hintingOptionsModel->appendRow(item);
ditto (and no space after *)
> fontsaasettings.cpp:70
> + FontAASettingsStore(FontsAASettings *parent = nullptr)
> + : QObject(parent)
> + , m_settings(parent)
Wrong indentation
> fontsaasettings.cpp:224
> + bool m_subPixelChanged;
> + int m_hinting;
> + bool m_hintingChanged;
I'd go for the enums all the way until here of course
> fontsaasettings.h:35
> + Q_PROPERTY(int dpi READ dpi WRITE setDpi NOTIFY dpiChanged)
> + Q_PROPERTY(int subPixel READ subPixel WRITE setSubPixel NOTIFY
> subPixelChanged)
> + Q_PROPERTY(int hinting READ hinting WRITE setHinting NOTIFY
> hintingChanged)
Why int here and for the next property and not the enum types directly? This
would avoid the static_cast in KRDB.
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D27156
To: bport, #plasma, ervin, crossi, meven
Cc: usta, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh,
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
