Braces around for, break early.
AcceptedPublic

Authored by tcanabrava on Dec 20 2019, 7:22 PM.

Details

Reviewers
ervin
dfaure
Test Plan

Recompiled, Run Unittests

Diff Detail

Repository
R237 KConfig
Branch
arcpatch-D26131
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21590
Build 21608: arc lint + arc unit
tcanabrava created this revision.Dec 20 2019, 7:22 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 20 2019, 7:22 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
tcanabrava requested review of this revision.Dec 20 2019, 7:22 PM
patrickelectric added inline comments.
src/kconfig_compiler/kconfig_compiler.cpp
178–179

std::any_of ?

ervin added a subscriber: ervin.Dec 23 2019, 5:26 PM
ervin added inline comments.
src/kconfig_compiler/kconfig_compiler.cpp
178–179

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

tcanabrava updated this revision to Diff 74258.Jan 23 2020, 4:33 PM
  • use std::any_of
ervin added a comment.Jan 28 2020, 4:13 PM

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

src/kconfig_compiler/kconfig_compiler.cpp
179

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?

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

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; });
ervin added a comment.Jan 29 2020, 6:51 AM

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

Indeed.

dfaure accepted this revision.Apr 14 2020, 8:46 PM

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.

This revision is now accepted and ready to land.Apr 14 2020, 8:46 PM