D27096: Refactoring, improve validation in kcm, optimize

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> charrunner.cpp:40
> +        Plasma::RunnerContext::ShellCommand);
> +    addSyntax(Plasma::RunnerSyntax(m_triggerWord + QStringLiteral(":q:"),
> +                                   i18n("Creates Characters from :q: if it is a hexadecimal code or defined alias.")));

m_triggerWord is empty when this executes. I think that needs to move back to reloadConfiguration

> charrunner.cpp:50
>  {
> -  KConfigGroup grp = config(); //Create config-object
> -
> -  m_triggerWord = grp.readEntry(CONFIG_TRIGGERWORD, "#"); //read out the triggerword
> -  m_aliases = grp.readEntry(CONFIG_ALIASES, QStringList());
> -  m_codes = grp.readEntry(CONFIG_CODES, QStringList());
> -  addSyntax(Plasma::RunnerSyntax(m_triggerWord + QLatin1String( ":q:" ),
> -                                 i18n("Creates Characters from :q: if it is a hexadecimal code or defined alias.")));
> +    const KConfigGroup &grp = config();
> +    m_triggerWord = grp.readEntry(CONFIG_TRIGGERWORD, DEFAULT_TRIGGERWORD);

unnecessary ref

> charrunner.cpp:90
>      context.addMatch(match);
>  }
> +void CharacterRunner::run(const Plasma::RunnerContext &context, const Plasma::QueryMatch &match)

newline

> charrunner_config.cpp:64
>      if (aliasList.size() == codeList.size()) {
> -        for (int i = 0; i < aliasList.size(); ++i) { //read out aliaslist and add Items to the list-view widget
> -            QTreeWidgetItem* item = new QTreeWidgetItem(m_ui->list, 2);
> +        for (int i = 0, size = aliasList.size(); i < size; ++i) {
> +            QTreeWidgetItem *item = new QTreeWidgetItem(m_ui->list, 2);

There is no purpose to this size variable. Compilers will optimize `i < aliasList.size()`.

> charrunner_config.cpp:72
> +        const auto msg = new KMessageWidget(
> +            QStringLiteral("Config entries for alias list and code list have different sizes, ignoring all."), this);
> +        m_ui->verticalLayout->insertWidget(0, msg);

this needs `i18nc()`

> charrunner_config.cpp:89
> +    QList<QString> codeList;
> +    for (int i = 0, count = m_ui->list->topLevelItemCount(); i < count; ++i) {
> +        aliasList.append(m_ui->list->topLevelItem(i)->text(0));

count not useful

> charrunner_config.cpp:120
> +    m_ui->list->takeTopLevelItem(m_ui->list->indexOfTopLevelItem(m_ui->list->currentItem()));
> +}
> +void CharacterRunnerConfig::validateAddButton()

newline

> charrunner_config.cpp:124
> +    m_ui->addItem->setDisabled(m_ui->edit_alias->text().isEmpty() || m_ui->edit_hex->text().isEmpty());
> +}
> +void CharacterRunnerConfig::validateDeleteButton()

newline

REPOSITORY
  R114 Plasma Addons

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

To: alex, davidedmundson, ngraham, sitter, broulik, #plasma
Cc: 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/20200210/2fdc2768/attachment-0001.html>