D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
ervin added inline comments.
INLINE COMMENTS
> KConfigHeaderGenerator.cpp:267
> +
> +for (const auto signal : qAsConst(parseResult.signalList)) {
> +stream() << whitespace() << " " << signalEnumName(signal.name) << "
> = 0x" << hexValue << val << ",\n";
You use "const Signal &" a few lines below, makes me wonder shouldn't they both
be "const auto &" then? at least for consistency?
REPOSITORY
R237 KConfig
BRANCH
remove_enum_hack
REVISION DETAIL
https://phabricator.kde.org/D26876
To: tcanabrava, ervin, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
tcanabrava added a comment. update / rebased. INLINE COMMENTS > ervin wrote in KConfigHeaderGenerator.cpp:252 > I lost track of constness here so I might be wrong, shouldn't this use > qAsConst in this context? yeah. REPOSITORY R237 KConfig BRANCH remove_enum_hack REVISION DETAIL https://phabricator.kde.org/D26876 To: tcanabrava, ervin, dfaure, apol Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
tcanabrava updated this revision to Diff 74596. tcanabrava marked an inline comment as done. tcanabrava added a comment. - Use qAsConst REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26876?vs=74253=74596 BRANCH remove_enum_hack REVISION DETAIL https://phabricator.kde.org/D26876 AFFECTED FILES autotests/kconfig_compiler/test13.h.ref autotests/kconfig_compiler/test_signal.h.ref src/kconfig_compiler/KConfigHeaderGenerator.cpp To: tcanabrava, ervin, dfaure, apol Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
ervin added inline comments.
INLINE COMMENTS
> KConfigHeaderGenerator.cpp:252
>
> -// HACK: Use C-Style for add a comma in all but the last element,
> -// just to make the source generated code equal to the old one.
> -// When we are sure, revert this to a range-based-for and just add
> -// a last comma, as it's valid c++.
> -for (int i = 0, end = parseResult.signalList.size(); i < end; i++) {
> -auto signal = parseResult.signalList.at(i);
> -stream() << whitespace() << " " << signalEnumName(signal.name) << "
> = 0x" << hex << val;
> -if (i != end-1) {
> -stream() << ",\n";
> -}
> -
> +for (const auto signal : parseResult.signalList) {
> +stream() << whitespace() << " " << signalEnumName(signal.name) << "
> = 0x" << hex << val << ",\n";
I lost track of constness here so I might be wrong, shouldn't this use qAsConst
in this context?
REPOSITORY
R237 KConfig
BRANCH
remove_enum_hack
REVISION DETAIL
https://phabricator.kde.org/D26876
To: tcanabrava, ervin, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
apol accepted this revision. This revision is now accepted and ready to land. REPOSITORY R237 KConfig BRANCH remove_enum_hack REVISION DETAIL https://phabricator.kde.org/D26876 To: tcanabrava, ervin, dfaure, apol Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
tcanabrava added reviewers: ervin, dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26876 To: tcanabrava, ervin, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26876: Remove the Enum hack: finish lists with a comma, it's valid c++
tcanabrava created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. tcanabrava requested review of this revision. TEST PLAN Run tests REPOSITORY R237 KConfig BRANCH remove_enum_hack REVISION DETAIL https://phabricator.kde.org/D26876 AFFECTED FILES autotests/kconfig_compiler/test13.h.ref autotests/kconfig_compiler/test_signal.h.ref src/kconfig_compiler/KConfigHeaderGenerator.cpp To: tcanabrava Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
