D16942: CMakeCacheModel: avoid string memory duplication

[Rolf Eike Beer]

dakon created this revision.
Herald added a project: KDevelop.
Herald added a subscriber: kdevelop-devel.
dakon requested review of this revision.

REVISION SUMMARY
QString::mid() will create a new string, but this code path will never use the
old variable again. So simply modify the original copy instead.

CMakeCacheModel: remove impossible case

Exactly the same case has been checked 2 if's before.

REPOSITORY
R32 KDevelop

BRANCH
cmcachemgr (branched from master)

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

AFFECTED FILES
plugins/cmake/settings/cmakecachemodel.cpp

To: dakon
Cc: kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Sven Brauch]

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


Looks ok to me, thanks!

REPOSITORY
R32 KDevelop

BRANCH
cmcachemgr (branched from master)

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

To: dakon, brauch
Cc: brauch, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Rolf Eike Beer]

This revision was automatically updated to reflect the committed changes.
Closed by commit R32:c4a0e378e980: CMakeCacheModel: avoid string memory duplication (authored by dakon).

CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D16942?vs=45624&id=45687#toc

REPOSITORY
R32 KDevelop

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16942?vs=45624&id=45687

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

AFFECTED FILES
plugins/cmake/settings/cmakecachemodel.cpp

To: dakon, brauch
Cc: brauch, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Milian Wolff]

mwolff added inline comments.

INLINE COMMENTS

cmakecachemodel.cpp:76
+ line.remove(0, 2);
+ currentComment += line;
+ } else if(!line.isEmpty() && !line.startsWith(QLatin1Char('#'))) //it is a variable

would be easier to just write

currentComment += line.midRef(2);

REPOSITORY
R32 KDevelop

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

To: dakon, brauch
Cc: mwolff, brauch, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Rolf Eike Beer]

dakon added inline comments.

INLINE COMMENTS

mwolff wrote in cmakecachemodel.cpp:76
would be easier to just write
currentComment += line.midRef(2);

Since "line" itself goes away after this loop there isn't anything to ref anymore, and I honestly don't know if this will work then. currentComment is a QStringList, no QString.

REPOSITORY
R32 KDevelop

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

To: dakon, brauch
Cc: mwolff, brauch, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Milian Wolff]

mwolff added a comment.


ah I see, then your code should indeed be better!

REPOSITORY
R32 KDevelop

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

To: dakon, brauch
Cc: mwolff, brauch, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd