D25449: Start of the new Formats KCM

[Kevin Ottens]

ervin requested changes to this revision.
ervin added a comment.
This revision now requires changes to proceed.


  Like the other ones, please use name and key in the entry elements of your kcfg files.

INLINE COMMENTS

> kcm.cpp:33
> +
> +Formats::Formats(QObject* parent, const QVariantList& args)
> +    : KQuickAddons::ManagedConfigModule(parent, args)

Ditto

> kcm.cpp:41
> +
> +    KAboutData* about = new KAboutData(
> +    QStringLiteral("kcm_formats"), i18n("Formats Configuration Module"),

Ditto, beside could be auto

> kcm.cpp:107
> +{
> +    // I don't know how to do this `save` within the KConfigXT
> +    auto config = KConfigGroup(KSharedConfig::openConfig("plasma-localerc"), "Formats");

Overriding usrSave in the settings object perhaps?

> kcm.cpp:161
> +}
> +#include "kcm.moc"

Missing an empty line before this one.

> kcm.h:38
> +public:
> +    Formats(QObject* parent, const QVariantList& args);
> +    virtual ~Formats() override;

Space before * and & not after

> localemodel.cpp:15
> +    // Ensure that the "Default (C)" locale always appears at the top
> +    if (c1.name()== QLatin1Char('C') && c2.name()!=QLatin1String("C")) return true;
> +    if (c2.name()== QLatin1Char('C')) return false;

Missing spaces around !=

> localemodel.cpp:16
> +    if (c1.name()== QLatin1Char('C') && c2.name()!=QLatin1String("C")) return true;
> +    if (c2.name()== QLatin1Char('C')) return false;
> +

Missing space before ==

> localemodel.cpp:36
> +
> +QString getCountryLocale(const QString& localeName)
> +{

Space before & and not after

> localemodel.cpp:51
> +
> +QString getFlagCode(const QString& localeName)
> +{

Ditto

> localemodel.cpp:62
> +
> +LocaleModel::LocaleModel(QObject* parent)
> +{

Ditto

> localemodel.cpp:69
> +
> +    for(auto locale : allLocales) {
> +        const QString clabel = !locale.nativeCountryName().isEmpty() ? locale.nativeCountryName() : QLocale::countryToString(locale.country());

for (const auto &localbe: qAsConst(allLocales))

> localemodel.cpp:83
> +
> +int LocaleModel::rowCount(const QModelIndex& parent) const
> +{

Space before & not after

> localemodel.cpp:89
> +
> +QVariant LocaleModel::data(const QModelIndex& idx, int role) const
> +{

Ditto

> localemodel.cpp:104
> +
> +int LocaleModel::indexFor(const QString& value) const
> +{

Ditto

> localemodel.cpp:106
> +{
> +    for(int i = 0; i < m_data.size(); i++) {
> +        if (m_data.at(i).localeValue == value) {

This sounds like a std::find_if

> localemodel.cpp:121
> +{
> +    return QHash<int, QByteArray> ({
> +        { Flag, "flag" },

You could just have return { here

> main.qml:34
> +
> +        QQC2.ComboBox {
> +            Kirigami.FormData.label: i18n("Region")

Shouldn't controls be deactivated based on the keys mutability here?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D25449

To: tcanabrava, ervin
Cc: ervin, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 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/20191212/23f398ef/attachment-0001.html>