Patch: TODO highlighting in comments
mail at milianw.de
Mon Oct 11 16:23:19 UTC 2010
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).
stripped_right += KDevelop::rStrip( "/**", *it );
doesn't look correct, /** is left, no? not sure about the API though , just a
- 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
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
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
//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.
mail at milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 198 bytes
Desc: This is a digitally signed message part.
More information about the KDevelop-devel