sitter added a comment.
Mh. Not quite what I had in mind but I suppose it makes sense this way.
I think we need a test case for the highlighter though :| It totally blows up
in my face when I trace a running dolphin because toskip isn't quite right.
INLINE COMMENTS
> backtracegenerator.cpp:95
>
> + auto preamble = new QTemporaryFile;
> + preamble->open();
This is leaking the file, is it not? It never deletes this object.
> gdbhighlighter.cpp:59
> int lineNr = currentBlock().firstLineNumber();
> + int toskip = 1 + m_infoLinesCount; // 1st line contains 'Application:
> ...'
> while ( cur < text.length() ) {
lineNr is initialized to currentBlock().firstLineNumber() that is not
necessarily 0, so toskip needs to be
`lineNr + 1 + infocount` otherwise the skipping doesn't work as expected.
Should also be camel toSkip.
> gdbhighlighter.cpp:65
> }
> - if (lineNr == 0) {
> - // line that contains 'Application: ...'
> + if (lineNr <= toskip) {
> ++lineNr;
Isn't this off-by-one versus the original code?
Say we have no infolines we'd then skip
[0] and [1] when previously we'd only skip [0].
> gdbhighlighter.cpp:76
> + // toskip since we skip the first line and the info lines
> + QMap< int, BacktraceLine >::iterator it = lines.lowerBound(lineNr -
> toskip);
> Q_ASSERT(it != lines.end());
The assert below fails when I trace a running dolphin, I am not super sure why
but I am guessing it's because the toskip init being bugged vis a vis the
lineNr being an offset.
REPOSITORY
R871 DrKonqi
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D28129
To: apol, #frameworks, broulik, sitter
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
