This revision was automatically updated to reflect the committed changes.
Closed by commit R39:f1d9f086da6f: KadeModeMenuList: fix memory leaks and
others (authored by nibags, committed by cullmann).
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D24210?vs=6
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.
Like that better, thanks!
REPOSITORY
R39 KTextEditor
BRANCH
fix-mode-menu
REVISION DETAIL
https://phabricator.kde.org/D24210
To: nibags, #ktexteditor, dhaumann, cullmann
Cc: kwr
nibags updated this revision to Diff 67026.
nibags added a comment.
- Fix allocation in search names
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D24210?vs=66864&id=67026
BRANCH
fix-mode-menu
REVISION DETAIL
https://phabricator.kde.org/D24210
AFF
cullmann requested changes to this revision.
This revision now requires changes to proceed.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D24210
To: nibags, #ktexteditor, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh,
n
cullmann added a comment.
I would prefer just some QString m_searchName, strings are shared anyways, no
need to try to save there a few bytes but then introduce more heap allocated
things.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D24210
To: nibags, #ktexte
nibags added a comment.
`m_searchName` is a pointer to an attribute of a KateFileType object (which
may be the string `translatedName` or `name`), but in some cases a new string
is created.
I added the boolean `m_bHasNewSearchName` that is true when a new QString is
created for search.
nibags updated this revision to Diff 66864.
nibags added a comment.
- Update
- Add boolean `m_bHasNewSearchName` to verify removal of `m_searchName`.
- Correct format in some comments.
- Remove `friend KateModeMenuList` and replace it with the "Factory" class.
REPOSITORY
R39 KTextEd
cullmann added a comment.
I must confess the m_searchName deletion condition seems for me too be overly
complex.
Given QString copying is cheap, can't we just always set that and store it
inline as value?
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D24210
T
nibags created this revision.
nibags added reviewers: KTextEditor, dhaumann, cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.
REVISION SUMMARY
- Remove strings `m_searchName` in the destruct
