Do not generate string list at runtime
ClosedPublic

Authored by kossebau on Sep 1 2019, 1:16 PM.

Details

Summary

GIT_SILENT

Diff Detail

Repository
R39 KTextEditor
Branch
preparelistatbuildtie
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15962
Build 15980: arc lint + arc unit
kossebau created this revision.Sep 1 2019, 1:16 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptSep 1 2019, 1:16 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
kossebau requested review of this revision.Sep 1 2019, 1:16 PM
cullmann requested changes to this revision.Sep 1 2019, 3:22 PM
cullmann added a subscriber: cullmann.

Hmm, could we here now avoid the QStringList completely and just use a normal array with a range based for below?

This revision now requires changes to proceed.Sep 1 2019, 3:22 PM

(And thanks for all the quick review :) )

Hmm, could we here now avoid the QStringList completely and just use a normal array with a range based for below?

Okay, looks like. Was trying to focus on one thing at a time, but you will get an update in a minute :)

kossebau updated this revision to Diff 65164.Sep 1 2019, 6:22 PM

now as plain array

cullmann accepted this revision.Sep 1 2019, 6:34 PM

I think that is nicer, yes!

This revision is now accepted and ready to land.Sep 1 2019, 6:34 PM
This revision was automatically updated to reflect the committed changes.
dhaumann added inline comments.
src/mode/katemodemanager.cpp
214–220 ↗(On Diff #65124)

I would even prefer:

static const auto commonSuffixes = { ... };

This way it's an initializer_list and a pattern we use at other places in KTextEditor as well. But of course, your patch is correct as well! Runtime should be the same.

kossebau added inline comments.Sep 1 2019, 10:20 PM
src/mode/katemodemanager.cpp
214–220 ↗(On Diff #65124)

I saw you mentioned this elsewhere, but thought it was a typo. Never seen this before, so curious to leatn what advantage using an initializer list brings here. By the name, i would expect its prpose is usually one-time, to init a bigger structure. But by the code it seems here the list is kept, and simply always iterated over?
Where is the motivation for this?

dhaumann added a subscriber: mwolff.Sep 2 2019, 5:43 AM
dhaumann added inline comments.
src/mode/katemodemanager.cpp
214–220 ↗(On Diff #65124)

Because you can write auto and don't have to care. The initializee_list has bwgin(), end(), size(), all which the plain C array does not have:
https://en.cppreference.com/w/cpp/utility/initializer_list

But I could imagine the generated code even looks the same? Maybe @mwolff knows.