D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[David Edmundson]

davidedmundson added a comment.


  My fault, I'll fix it

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex added a comment.


  Sh**t, should I make a new patch to fix this?
  
  And sorry ...

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Nathaniel Graham]

ngraham added a comment.


  This broke the CI:
  
[2020-03-24T15:59:18.523Z] [ 66%] Linking CXX shared module 
../../bin/krunner_datetime.so
[2020-03-24T15:59:18.523Z] 
/usr/home/jenkins/workspace/Plasma/kdeplasma-addons/kf5-qt5 
FreeBSDQt5.14/runners/spellchecker/spellcheck.cpp:198:30: error: use of 
undeclared identifier 'findlang'; did you mean 'findLang'?
[2020-03-24T15:59:18.523Z] const QString lang = findlang(terms);
[2020-03-24T15:59:18.523Z] ^~~~

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:5e760a0586bb: Refactor foreach, use Qt5 signal syntax, 
remove unnecessary method call (authored by davidedmundson, committed by alex).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=78368&id=78371

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex added a comment.


  Thanks!

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[David Edmundson]

davidedmundson updated this revision to Diff 78368.
davidedmundson added a comment.


  Rebase

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=78336&id=78368

BRANCH
  master

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex added a comment.


  Small issue: Currently this patch can't be applied (from the master branch)  
because in R114:47381a65debe01a0a015e1aa423cbb78af649648 
 the 
line:
  `QStringList terms = query.split(QLatin1Char(' '), QString::SkipEmptyParts);` 
got changed.
  I don't know how to resolve this conflict and would be thankful for help :-).

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 78336.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=78335&id=78336

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 78335.
alex added a comment.


  Minor improvements
  
  I had another look at the project and changed some other things:
  
  - optimize some overloads with QStringLiteral/QLatin1String
  - rename loaddata to loadData
  - remove the unnecessary check for the action in the run method
  - reuse the result of config()
  - make sure that the trigger word can only be required if it is not empty

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74848&id=78335

BRANCH
  arcpatch-D27070

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

AFFECTED FILES
  CMakeLists.txt
  runners/converter/CMakeLists.txt
  runners/converter/autotests/CMakeLists.txt
  runners/converter/autotests/converterrunnertest.cpp
  runners/converter/converterrunner.cpp
  runners/converter/converterrunner.h
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 74848.
alex added a comment.


  Undo changes that were supposed to be in new diff

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74847&id=74848

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 74847.
alex added a comment.


  Refactoring, improve validation in kcm, optimize

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74830&id=74847

BRANCH
  charrunner_fix

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

AFFECTED FILES
  runners/characters/charrunner.cpp
  runners/characters/charrunner.h
  runners/characters/charrunner_config.cpp
  runners/characters/charrunner_config.h
  runners/characters/charrunner_config.ui
  runners/characters/config_keys.h
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex marked an inline comment as done.
alex added a comment.


  Hello,
  thanks for the response.
  
  I addition to your requested change  I have made some smaller improvements.
  
  But I have a question about the concept of this runner: Why is the data for 
every match session newly created ? I understand that it saves a bit of memory 
to delete them, but this way the spellers are for every match session created
  (and the prepare/teardown slots are called in the main thread).
  
  What do you think about loading the data in the reloadConfiguration method 
and then reusing it ?
  This way the spellers for new languages can be also reused for multiple match 
sessions (they are in the match method created).
  
  PS: When I tested this plugin I found out that the "task-attention" icon for 
the error message (line 252 in the runner) is no longer available, but I don't 
know which icon would be a good replacement.

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 74830.
alex added a comment.


  Simplify and optimize

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74795&id=74830

BRANCH
  master

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck.h
  runners/spellchecker/spellcheck_config.cpp

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> spellcheck.cpp:145
>  {
> -auto defaultSpeller = m_spellers[QString()];
> +auto &defaultSpeller = m_spellers[QString()];
>  //If first term is a language code (like en_GB), set it as the 
> spell-check language

Const auto &

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

To: alex, #plasma, ngraham, broulik, davidedmundson
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex added reviewers: ngraham, broulik.
alex added a comment.


  > Sorry. I'm removing myself as a reviewer. I barely remember this code base. 
It has been way too many years.
  
  Thanks anyway.

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Vishesh Handa]

vhanda added a comment.


  Sorry. I'm removing myself as a reviewer. I barely remember this code base. 
It has been way too many years.

REPOSITORY
  R114 Plasma Addons

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

To: alex, #plasma
Cc: vhanda, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 74795.
alex added a comment.


  Simplify statement

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74793&id=74795

BRANCH
  master

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck_config.cpp

To: alex, kossebau, vhanda
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex updated this revision to Diff 74793.
alex added a comment.


  Remove unused include

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27070?vs=74789&id=74793

BRANCH
  master

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck_config.cpp

To: alex, kossebau, vhanda
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27070: Refactor foreach, use Qt5 signal syntax, remove unnecessary method call

[Alexander Lohnau]

alex created this revision.
alex added reviewers: kossebau, vhanda.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
alex requested review of this revision.

REVISION SUMMARY
  The two foreach occurences are refactored and the signals in the runner/kcm 
now use the C++ 11 for loop.
  Additonally the duplicate call of the load method has been removed and 
smaller improvements have been made.

TEST PLAN
  Compile time check for signals, trying out the runner and editing the config.

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp
  runners/spellchecker/spellcheck_config.cpp

To: alex, kossebau, vhanda
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart