Patch: TODO highlighting in comments

[Dmitry Risenberg]

The patch itself.

2010/10/14 Dmitry Risenberg <dmitry.risenberg at gmail.com>:
>> 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
>>
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdevelop.patch
Type: application/octet-stream
Size: 8906 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20101014/c2a7bfae/attachment.obj>