Patch: TODO highlighting in comments

[Milian Wolff]

On Monday 11 October 2010 13:24:19 Dmitry Risenberg wrote:
> Looks like the previous message was overlooked, so re-posting it.
> 
> Here are the patches that provide searching for TODOs in comments,
> highlighting them as semantic problems and reporting via the
> problemview plugin.

review (btw.: if you could create a merge request, it would make it much 
easier for us to do the review and merge it afterwards. please do so).

- cpp/commentformatter.cpp:

stripped_right += KDevelop::rStrip( "/**", *it );

doesn't look correct, /** is left, no? not sure about the API though , just a 
guess

- cpp/parser.cpp contains an added newline in addComment, please remove

- imo all these errors should be reported, you currently check 
_M_max_problem_count which is for parser errors, if you got a file with more 
than five FIXMEs, only the first five will be shown. Not good imo

- findToDos with a QRegExp is too slow for my taste, you should use simple 
.contains() here.

or better yet, make a helper where you pass in the whole comment and let it 
figure out what to report. imo this should be how it's done:

/**
 * FIXME: short description that should be reported in the error
 * arbitrary other stuff that should not be included
 * TODO: some second problem
 */

So the helper should report (possibly more than one) todo per comment, but 
only one per line. Why? think about this:

/* FIXME: this does not work
  myNullPtr()->derefAndCrash();
*/

I only want to see the first line, not the second

So just iterate over the (left-stripped) lines in the comment and see whether 
they start with FIXME or TODO then report a problem there.

Bonus points if this is working:

/* FIXME: rewrite this mess
  oldCode();
  //TODO: outdated, don't show me
*/

> There is a known issue with adjacent comments, like
> 
> /* TODO */ /* FIXME */
> int i;
> 
> - it will underline "TODO */ /* FIXME" part, which is wrong. This is
> because /* TODO */ /* FIXME */ is seen as one token by the parser -
> not sure whether it should be this way.

see above, imo this can be ignored. One problem per line and you are done. Who 
is using this format anyways?

> Also haven't measured the influence this change has on performance
> (don't know if there is a simple way).

duchainify --force-update-recursive /path/to/some/big/project

before and after in time, then maybe one more run with callgrind after the 
patch and see how much time is spent in the new functions.

bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20101011/944dd3f3/attachment.sig>