Patch: TODO highlighting in comments

[Dmitry Risenberg]

> 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).

Cannot create the request for kdevelop repository, gitorous says "This
Git object is too large to be displayed in the browser" or just fails
with a 503 error. Merge request to kdevplatform went fine though.

2010/10/11 Milian Wolff <mail at milianw.de>:
> 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
>
> --
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>
>