Patch: TODO highlighting in comments

Dmitry Risenberg dmitry.risenberg at gmail.com
Thu Oct 14 20:27:52 UTC 2010


Gitorious still not working, so sending here the updated patch.

- Removed _M_problem_count from patch.
- Could not replace iterators with const iterators, because
strip/rStip functions take a non-const reference as a parameter. IMHO
this could be fixed by returning the processed value instead of
modifying the parameter, but this is an API change.

2010/10/14 Dmitry Risenberg <dmitry.risenberg at gmail.com>:
> 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: 7505 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20101015/ad128d8e/attachment.obj>


More information about the KDevelop-devel mailing list