Use QString::count(QChar) over QString::split().length() - 1
ClosedPublic

Authored by kossebau on Sep 1 2019, 10:37 AM.

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Sep 1 2019, 10:37 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptSep 1 2019, 10:37 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
kossebau requested review of this revision.Sep 1 2019, 10:37 AM
cullmann accepted this revision.Sep 1 2019, 3:11 PM
cullmann added a subscriber: cullmann.

Looks reasonable, thanks.

This revision is now accepted and ready to land.Sep 1 2019, 3:11 PM
This revision was automatically updated to reflect the committed changes.
apol added a subscriber: apol.Sep 2 2019, 11:23 AM
apol added inline comments.
src/vimode/modes/normalvimode.cpp
3844

you didn't port this one to count?

kossebau added inline comments.Sep 2 2019, 11:31 AM
src/vimode/modes/normalvimode.cpp
3844

Guess the complete change missed to enter the commit, and review did not catch it,
Well spotted, I shall do a follow-up.

kossebau added inline comments.Sep 2 2019, 11:42 AM
src/vimode/modes/normalvimode.cpp
3844

Ah, no, textLines.last().length() needs the string section, so just a plain count would not be enough, only if combining with a QString::section call later, I now remember.
And some other patch simply turned this into splitRef, so just one alloc here currently.

So rather something where I messed up during patch juggling.

Perhaps still can be µ-optimized with using lastindexof instead :)

kossebau added inline comments.Sep 2 2019, 12:04 PM
src/vimode/modes/normalvimode.cpp
3844

Given we talked about it and my brain has created an item, D23674 is to get rid of it again :)