D26131: Braces around for, break early.

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  I'd remove the space after '!' and make the bool const, but I'm nitpicking, 
you can also push as is.
  
  cbegin/cend wouldn't change anything on a const container.

REPOSITORY
  R237 KConfig

BRANCH
  arcpatch-D26131

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

To: tcanabrava, ervin, dfaure
Cc: kossebau, ervin, patrickelectric, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D26131: Braces around for, break early.

[Kevin Ottens]

ervin added a comment.


  In D26131#602149 , @dfaure wrote:
  
  > No, never const QChar&.
  >  QChar is a short int.
  
  
  Indeed.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, ervin, dfaure
Cc: kossebau, ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D26131: Braces around for, break early.

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Getting rid of the inversion might be easier to read for simple humans (as me 
:) ):
  
const bool isAscii = std::all_of(std::begin(s), std::end(s), [](QChar a) { 
return a.unicode() <= 127; });

REPOSITORY
  R237 KConfig

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

To: tcanabrava, ervin, dfaure
Cc: kossebau, ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, ngraham, bruns

D26131: Braces around for, break early.

[David Faure]

dfaure added a comment.


  No, never const QChar&.
  QChar is a short int.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, ervin, dfaure
Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26131: Braces around for, break early.

[Kevin Ottens]

ervin added a comment.


  Does it apply on top of your refactoring? Also the description looks wrong 
now.

INLINE COMMENTS

> kconfig_compiler.cpp:179
>  {
> -bool isAscii = true;
> -for (int i = s.length(); i--;)
> -if (s[i].unicode() > 127) {
> -isAscii = false;
> -}
> -
> +bool isAscii = ! std::any_of(std::begin(s), std::end(s), [](QChar a) { 
> return a.unicode() > 127; });
>  if (isAscii) {

Nitpick: there shouldn't be a space after !

Also: use std::cbegin and std::cend (I think they're allowed nowadays)
And: "const QChar &" instead of QChar?

REPOSITORY
  R237 KConfig

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

To: tcanabrava, ervin, dfaure
Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26131: Braces around for, break early.

[Tomaz Canabrava]

tcanabrava added reviewers: ervin, dfaure.

REPOSITORY
  R237 KConfig

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

To: tcanabrava, ervin, dfaure
Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26131: Braces around for, break early.

[Tomaz Canabrava]

tcanabrava updated this revision to Diff 74258.
tcanabrava added a comment.


  - use std::any_of

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26131?vs=71913=74258

BRANCH
  arcpatch-D26131

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

AFFECTED FILES
  src/kconfig_compiler/kconfig_compiler.cpp

To: tcanabrava
Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26131: Braces around for, break early.

[Kevin Ottens]

ervin added inline comments.

INLINE COMMENTS

> patrickelectric wrote in kconfig_compiler.cpp:619
> std::any_of ?

Indeed, could be replaced by any_of since we're at it.

REPOSITORY
  R237 KConfig

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

To: tcanabrava
Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26131: Braces around for, break early.

[patrick j pereira]

patrickelectric added inline comments.

INLINE COMMENTS

> kconfig_compiler.cpp:619
>  bool isAscii = true;
> -for (int i = s.length(); i--;)
> +for (int i = s.length(); i--;) {
>  if (s[i].unicode() > 127) {

std::any_of ?

REPOSITORY
  R237 KConfig

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

To: tcanabrava
Cc: patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26131: Braces around for, break early.

[Tomaz Canabrava]

tcanabrava created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
tcanabrava requested review of this revision.

TEST PLAN
  Recompiled, Run Unittests

REPOSITORY
  R237 KConfig

BRANCH
  nitpicksLiteralString

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

AFFECTED FILES
  src/kconfig_compiler/kconfig_compiler.cpp

To: tcanabrava
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns