D27156: KCM Fonts port anti aliasing part to KPropertySkeletonItem

[Kevin Ottens]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200211/a460fe75/attachment-0001.html>