Bug 79459 - Tasks not working
Summary: Tasks not working
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 3.0   Edit
Hardware: PC Linux
: P3 enhancement with 27 votes (vote)
Target Milestone: 4.0 RC3   Edit
Assignee: Markus Schorn CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
: 80663 147097 171404 (view as bug list)
Depends on: 42916 148360 183930
Blocks: 39478
  Show dependency tree
 
Reported: 2004-11-24 20:16 EST by João Gonçalves CLA
Modified: 2008-06-22 01:10 EDT (History)
21 users (show)

See Also:


Attachments
Work in progress (24.20 KB, patch)
2007-04-13 04:43 EDT, Sergey Prigogin CLA
no flags Details | Diff
Working version (149.83 KB, patch)
2007-05-01 20:48 EDT, Sergey Prigogin CLA
no flags Details | Diff
Complete implementation (155.57 KB, patch)
2007-05-06 20:11 EDT, Sergey Prigogin CLA
no flags Details | Diff
Improved cleaner implementation (88.92 KB, patch)
2007-05-11 05:29 EDT, Sergey Prigogin CLA
no flags Details | Diff
Updated to keep compatible with HEAD (89.07 KB, patch)
2007-05-13 20:46 EDT, Sergey Prigogin CLA
no flags Details | Diff
Addressed Markus' comments (87.67 KB, patch)
2007-05-15 03:24 EDT, Sergey Prigogin CLA
no flags Details | Diff
Updated patch (85.20 KB, patch)
2007-05-29 00:26 EDT, Sergey Prigogin CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description João Gonçalves CLA 2004-11-24 20:16:05 EST
Hi all,

When I do //TODO  I have to do something  or /* TODO  i have to do something*/ .
In the tasks border it doesn't show anything. My friends report to me the same.
When I did a java project tasks bordar was working, but when i use CTD plugin
for a C project, me and my friends can't use the tasks border.
Comment 1 Kim Lux CLA 2005-11-29 15:36:20 EST
I've got the same problem.  TODOs not showing in tasks in CDT.  They highlight properly as a recognized word, but they don't show up in the tasks view.

Am I missing something ?

I need this badly. 
Comment 2 Doug Schaefer CLA 2005-11-29 15:44:51 EST
There were some problems with this feature so it was turned off (although I see only partially). I can't remember the details but if someone wants to take another look at this, there is demand for this.
Comment 3 Kim Lux CLA 2005-11-29 15:48:50 EST
WOW, that was a fast answer.  

I think this is a duplicate of bug 80663.

How much work and how difficult would it be to fix.  I've never worked on an Eclipse plugin, but I just received a code project with a bunch of TODO and FIXMEs in it. 
Comment 4 Doug Schaefer CLA 2005-11-29 15:53:25 EST
There's always a learning curve before getting started writing for Eclipse. But there are books out there that should help.

I just don't see anyone else looking at this issue at the moment so it won't get fixed any time soon without help.
Comment 5 Riku Leino CLA 2005-12-16 21:15:15 EST
Really missing this feature
Comment 6 Sebastian Huss CLA 2006-01-30 11:19:52 EST
It was a really negative suprise, because I was used to use this feature in the JDT, and I a kind of rely on this feature. Furthermore, there still is a "C/C++ Task"-option field in the Tasks-Filters-dialog.
Comment 7 arne anka CLA 2006-02-15 04:16:08 EST
regarding comment #2: are there infos available which kind of problems and where to start fixing? i would like to look into it if it's not overly complex (albeit i never touched eclipse-code before).
Comment 8 Kim Lux CLA 2006-02-15 12:42:22 EST
I'll reafirm that I am very interested in seeing this fixed.  I've got projects littered with TODOs in them that are a nuisance to view right now.   

I looked for info regarding stuff being turned off in the source code, but couldn't find anything. (I know very little about the internals in Eclipse.)
Comment 9 Markus Schorn CLA 2006-06-19 04:25:08 EDT
*** Bug 147097 has been marked as a duplicate of this bug. ***
Comment 10 Anton Leherbauer CLA 2006-09-11 11:09:05 EDT
*** Bug 80663 has been marked as a duplicate of this bug. ***
Comment 11 Andrey Loskutov CLA 2006-10-14 17:46:03 EDT
I'm wondering why this bug belongs to the CDT.

Basically, *any* text with TODO's should automatically generate todo markers - xml, jsp, js, java, c, h, properties, makefile etc - so why not to move it to the platform text? In the past I've filled similar TODO's bug reports for web tooling etc, but I think that this feature should not be implemented in each and every toolkit we have again and again. I could imagine, that then all the different settings for TODO generation you have in each toolkit (JDT, WST, CDT etc) for todo's may be moved to single place where it could be set consistently across all types of sources. 

The simpliest implementation may just have a common builder to scan the files for TODO's in a "plain text", because I think no one have variables with such name, so there is no need in building AST to see if it is a valid place for comment etc. 

On the other hand, if we would like to have "sophisticated" AST based solution, some compiler specific API/extension points may be added so that the compiler could contribute their domain knowledge to this TODO builder for specific content types. For text files without any compiler support I would just use plain text search.

So why not to move it to the platform text and made it 3.3 target?
Comment 12 Doug Schaefer CLA 2006-10-18 14:41:59 EDT
Well, I certianly don't want my variables called TODO to be flagged so you do need to at least know that you are in a comment. So knowledge of the language you are using is necessary.

At any rate, it won't happen in the Platform or the CDT without someone contributing a patch.
Comment 13 João Gonçalves CLA 2006-10-18 17:42:30 EDT
(In reply to comment #11)
> I'm wondering why this bug belongs to the CDT.
> 
> Basically, *any* text with TODO's should automatically generate todo markers -
> xml, jsp, js, java, c, h, properties, makefile etc - so why not to move it to
> the platform text? In the past I've filled similar TODO's bug reports for web
> tooling etc, but I think that this feature should not be implemented in each
> and every toolkit we have again and again. I could imagine, that then all the
> different settings for TODO generation you have in each toolkit (JDT, WST, CDT
> etc) for todo's may be moved to single place where it could be set consistently
> across all types of sources. 
> 
> The simpliest implementation may just have a common builder to scan the files
> for TODO's in a "plain text", because I think no one have variables with such
> name, so there is no need in building AST to see if it is a valid place for
> comment etc. 
> 
> On the other hand, if we would like to have "sophisticated" AST based solution,
> some compiler specific API/extension points may be added so that the compiler
> could contribute their domain knowledge to this TODO builder for specific
> content types. For text files without any compiler support I would just use
> plain text search.
> 
> So why not to move it to the platform text and made it 3.3 target?
> 


I agree!! Todo's must be done in the platform text. Saves times.. saves bugs.. and repeated code in all toolkits!!
Comment 14 Anton Leherbauer CLA 2007-01-23 11:03:54 EST
*** Bug 171404 has been marked as a duplicate of this bug. ***
Comment 15 bartosz michalik CLA 2007-04-13 02:44:10 EDT
I think I can look at this bug, but until it is not clear if this functionality should belong to CDT or to the text editors layer I don't know how.

Comment 16 Sergey Prigogin CLA 2007-04-13 03:11:11 EDT
(In reply to comment #15)
Scanning for TODO tags in existing CDT parser would have a lesser run time overhead than doing it independently in the platform, but most importantly it can be done sooner than if we wait for the platform. I already have some code written and hope to finish it before RC1. Will gladly share what I've done so far with Bartosz or anyone who has spare time to work on this.
Comment 17 Sergey Prigogin CLA 2007-04-13 04:43:33 EDT
Created attachment 63700 [details]
Work in progress

Here is what I got so far.
Comment 18 Markus Schorn CLA 2007-04-13 05:56:21 EDT
Bug 42916 indicates that the feature was originally provided by the CModelBuilder. I think it is better to have the indexer provide this information. The indexer processes a well defined set of files, while
the CModel is built for files on demand, only.

For the refactoring work comments in the AST are needed. Actually there
is a patch for this available. I suggest to let the indexer generate AST
containing the comments and generated the tasks off of this.
Comment 19 Sergey Prigogin CLA 2007-04-13 08:27:02 EDT
(In reply to comment #18)
This is exactly the path I'm taking.
Comment 20 Markus Schorn CLA 2007-04-13 09:59:02 EDT
(In reply to comment #19)
> (In reply to comment #18)
> This is exactly the path I'm taking.

Great! We have to make sure that the way the comments are represented in the AST meets the needs of the refactoring guys, also. So this part should be discussed in bug 148360. 
Comment 21 Warren Paul CLA 2007-04-18 15:42:13 EDT
Our users are asking for this as well.  It looks like there's a patch for this but it's not clear if it's complete.  Is this planned for the final CDT 4.0?
Comment 22 Sergey Prigogin CLA 2007-04-18 16:00:26 EDT
(In reply to comment #21)
The patch is far from complete, just some fragments of a solution.

Comment 23 João Gonçalves CLA 2007-04-19 17:13:21 EDT
I am happy to see this bug resolved! Since I report this bug that many people asked for some solution and I want to say: "Thanks to everyone involved".
Comment 24 Sergey Prigogin CLA 2007-05-01 20:48:45 EDT
Created attachment 65552 [details]
Working version

I got tasks working. Two things are still missing:

1. Changing task settings does not trigger index rebuild. I wonder if task tag settings should be included in IndexerPreferences.
2. No tests yet.
Comment 25 Sergey Prigogin CLA 2007-05-06 20:11:10 EDT
Created attachment 66040 [details]
Complete implementation

Toni or Markus,
Please review the patch and apply it, if there are no objections.

Change of task tag preferences now triggers an index rebuild that refreshes the markers. Added a test for tag parsing.
Comment 26 Markus Schorn CLA 2007-05-07 03:23:54 EDT
(In reply to comment #25)
> Created an attachment (id=66040) [details]
> Complete implementation
> Toni or Markus,
> Please review the patch and apply it, if there are no objections.
> Change of task tag preferences now triggers an index rebuild that refreshes the
> markers. Added a test for tag parsing.

Sergey, we are beyond feature freeze, so this will not make it into 4.0. Nevertheless I had a quick look at the patch. You should change the implementation such that it acesses the comments via IASTTranslationUnit.getComments().
Comment 27 Sergey Prigogin CLA 2007-05-07 13:20:11 EDT
(In reply to comment #26)
> You should change the implementation such that it acesses the comments via
IASTTranslationUnit.getComments().

Markus, I don't think this approach is going to work for a number of reasons:

1. IASTComment does not contain comment location.
2. Comments are not always awailable in the translation unit since they are
   generated conditionally.
3. Header files are not translation units, but they may contain task tags.
   Indexer skips previously indexed header files and does not expose a list
   of skipped files. Consider the following scenario. s1.cc and s2.cc both
   include i.h, which contains a TODO. During a full index rebuild s2.cc's
   ASTTranslationUnit won't have comments from i.h since it was skipped.
   Without knowing that i.h was skipped, the task extractor would iterate over
   all includes in s2.cc and erase task markers in i.h created while parsing
   s1.cc.

In fact, the semantics of IASTTranslationUnit.getComments() is not clear to me. Does it return comments from included header files? If so, how does it handle the situation when some included header files are skipped during parsing?

Is it possible to give the task tags feature an exemption from feature freeze due to high popular demand?
Comment 28 Sergey Prigogin CLA 2007-05-07 13:32:49 EDT
Please disregard "IASTComment does not contain comment location" reason, which is invalid. The other two reasons still hold.
Comment 29 Markus Schorn CLA 2007-05-08 02:50:17 EDT
> 2. Comments are not always awailable in the translation unit since they are
>    generated conditionally.
To get the comments the indexer has to supply the flag AbstractLanguage.OPTION_ADD_COMMENTS. You can supply the option in PDOMIndexerTask.internalParseTUs().

> 3. Header files are not translation units, but they may contain task tags.
>    Indexer skips previously indexed header files and does not expose a list
>    of skipped files. Consider the following scenario. s1.cc and s2.cc both
>    include i.h, which contains a TODO. During a full index rebuild s2.cc's
>    ASTTranslationUnit won't have comments from i.h since it was skipped.
>    Without knowing that i.h was skipped, the task extractor would iterate over
>    all includes in s2.cc and erase task markers in i.h created while parsing
>    s1.cc.
> In fact, the semantics of IASTTranslationUnit.getComments() is not clear to me.
> Does it return comments from included header files? If so, how does it handle
> the situation when some included header files are skipped during parsing?
> Is it possible to give the task tags feature an exemption from feature freeze
> due to high popular demand?

A translation unit is the source-file together with all the headers included. You get the comments from the source-file + the heades, which is the same semantics as when you obtain the declarations of a translation-unit.

A header is typically skipped in case it has already been indexed. So the comments have been analyzed before as well. 

Only when you use the preference 'indexer/indexAllFiles=false' those headers that are not included by any source-file are not indexed at all. Ignoring those with respect to task-tags seems to be consistent, however this could be a problem.
Comment 30 Sergey Prigogin CLA 2007-05-08 03:46:03 EDT
(In reply to comment #29)
> A header is typically skipped in case it has already been indexed. So the
> comments have been analyzed before as well. 

Are the comments from the skipped included files going to be returned by IASTTranslationUnit.getComments() or not?

The code updating tasks markers has to delete old ones from each file before creating new ones. It's not sufficient to delete old task markers from the files currently containing task tags. If a file does not contain task tags, but did contain them before, the old task markers would be left behind. IASTTRanslationUnit exposes a list of includes, but that list cannot be used for removing old task markers either. That would cause deletion of task markers from include files that do contain task tags but were skipped while parsing the last translation unit. TodoTaskReporter solves this problem by getting a notification from BaseScanner for each file that gets parsed.

IASTTRanslationUnit does not seem to contain enough information to correctly clean up the old task markers. Any suggestions will be appreciated.

Comment 31 Markus Schorn CLA 2007-05-08 04:22:11 EDT
(In reply to comment #30)
> Are the comments from the skipped included files going to be returned by
> IASTTranslationUnit.getComments() or not?
no, for skipped files the AST does not contain comments (just as it does not 
contain declarations).

> The code updating tasks markers has to delete old ones from each file before
> creating new ones. It's not sufficient to delete old task markers from the
> files currently containing task tags. If a file does not contain task tags, 
> but did contain them before, the old task markers would be left behind....

the very same problem has to be solved for indexing and is handled by the class PDOMWriter. I suggest to insert a hook into PDOMWriter.addSymbols() that allows the derived class PDOMIndexerTask to do handle the comments right after the index was updated for the files contained in one translation-unit. As arguments you can pass the IASTTranslationUnit and a list of affected IndexFileLocations. The list is available in the local variable 'orderedPaths' of 'PDOMWriter.addSymbols()'.
Comment 32 Sergey Prigogin CLA 2007-05-10 04:28:48 EDT
(In reply to comment #31)
Markus,
I reimplemented task parsing based on IASTTranslationUnit.getComments(), but ran into a problem. ASTTranslationUnit.getComments() returns ASTComment objects that have null fileLocation and locations fields. What is the right way to determine filename and line number of each comment?
Comment 33 Emanuel Graf CLA 2007-05-10 07:50:21 EDT
(In reply to comment #32)
> ASTTranslationUnit.getComments() returns ASTComment objects
> that have null fileLocation and locations fields. What is the right way to
> determine filename and line number of each comment?

That's a bug. I created a report for this. https://bugs.eclipse.org/bugs/show_bug.cgi?id=186337

Comment 34 Sergey Prigogin CLA 2007-05-11 05:29:02 EDT
Created attachment 66828 [details]
Improved cleaner implementation

Here is a more condense task tag implementation that obtains comments using ASTTranslationUnit.getComments() method. It depends on patch attached to bug 186337.

Please review and apply this patch so that masses can enjoy task tags in 4.0 instead of waiting for another year. Exemption from the feature freeze can be justified by:
1. High demand
2. Low risk
3. A fact that this is not a new feature, just a resurrection of an old broken one.
Comment 35 arne anka CLA 2007-05-11 05:49:56 EDT
i strongly second that!
Comment 36 Anton Leherbauer CLA 2007-05-11 06:00:51 EDT
(In reply to comment #34)
> Please review and apply this patch so that masses can enjoy task tags in 4.0
> instead of waiting for another year. Exemption from the feature freeze can be
> justified by:
> 1. High demand
> 2. Low risk
> 3. A fact that this is not a new feature, just a resurrection of an old broken
> one.
> 

I see three reasons against applying:
1. It _is_ a new feature, although it existed in earlier versions.
2. I suppose the patch would need to go through IP review first.
3. We need some great feature highlights for the next release, too ;-)
Comment 37 Jens Seidel CLA 2007-05-11 06:28:56 EDT
> I see three reasons against applying:

Oj, no, Please, please apply it for 4.0. There will be many testers since this is a very important feature and I also prominse to test it very carefully.

Otherwise I will probably continue usin daily build even after the 4.0 release just to get this feature ...

Jens
Comment 38 Sergey Prigogin CLA 2007-05-11 11:29:36 EDT
(In reply to comment #36)
I hope , there shouldn't be a problem with #2 since my other contribution was cleared by IP less than a year ago.
Comment 39 Mike Kucera CLA 2007-05-11 11:38:47 EDT
My opinion is that this feature should be included in 4.0 if at all possible. 

Tasks is a feature of JDT that I use almost every day. I think many users will
be disappointed if this feature doesn't work in CDT 4.0.
Comment 40 Chris Recoskie CLA 2007-05-11 11:51:38 EDT
This bug has 30 votes and is our #1 voted bug.  For a while this was one of the top ten most hated bugs on Eclipse.org.  Although it is late in the game I would be inclined say that we should bend the rules and put it in to make the community happy.  If we are worried about stability we can leave it turned off by default.
Comment 41 Doug Schaefer CLA 2007-05-11 12:25:10 EDT
Well, there are a couple of ways to look at this. First, it really isn't a new feature. The TODO feature has been broken now for a couple of years. You could see this as a bug fix.

And, since it hasn't worked for a couple of years, people aren't going to be any more disappointed than they were previously, so I'm not considering that in my opinion.

So I'm O.K. with it going in as a big bug fix. However, it is well large enough to require an IP review. If that can be completed in the next couple of weeks than I'd say go for it. If not, then I think it becomes to risky due to lack of test time. However, I'd think it could then make the maintenance release in Sept.
Comment 42 Sergey Prigogin CLA 2007-05-13 20:46:23 EDT
Created attachment 66999 [details]
Updated to keep compatible with HEAD

Could please somebody initiate an IP review. Thanks.

In meantime I've updated the patch to keep it compatible with HEAD.
Comment 43 Markus Schorn CLA 2007-05-14 09:56:37 EDT
I had a look at the integration with the indexer in your latest patch. It looks good! A few minor things caught my attention:
* IProblem.TASK seems to be the wrong place to define the constant for the marker.
* ParserMessages should contain messages for the parser, only.
* TodoTaskUpdater contains Java 1.5 code (Boolean.parseBoolean())
* PDOMWriter has a private method that is never called.

I'll initiate the IP-review.
Comment 44 Sergey Prigogin CLA 2007-05-15 03:24:45 EDT
Created attachment 67189 [details]
Addressed Markus' comments

(In reply to comment #43)
> * IProblem.TASK seems to be the wrong place to define the constant for the
> marker.
Marker "id" attribute does not seem to be used anywhere. Removed.
> * ParserMessages should contain messages for the parser, only.
Moved to the proper Messages file.
> * TodoTaskUpdater contains Java 1.5 code (Boolean.parseBoolean())
Fixed.
> * PDOMWriter has a private method that is never called.
Removed.
Comment 45 Markus Schorn CLA 2007-05-15 03:40:05 EDT
I have initiated the IP-review process:
https://dev.eclipse.org/ipzilla/show_bug.cgi?id=1535
Comment 46 Doug Schaefer CLA 2007-05-28 16:12:32 EDT
(In reply to comment #45)
> I have initiated the IP-review process:
> https://dev.eclipse.org/ipzilla/show_bug.cgi?id=1535

The request has been approved.
Comment 47 Sergey Prigogin CLA 2007-05-29 00:26:38 EDT
Created attachment 68996 [details]
Updated patch

Resolved conflicts with HEAD. Ready to be applied.
Comment 48 Markus Schorn CLA 2007-05-30 07:10:24 EDT
Thanks Sergey, I have applied the patch!

* I made modifications in the PDOMManager, because the patch bypassed the   
  synchronization for rebuilding the index.
* Also I fixed a problem with the parser when generating AST with comments. 



Comment 49 selso CLA 2007-06-25 04:52:57 EDT
I have read in "Linux Developper's journal" that some plugins insert TODO comments tags in the task view. The C/C++ Editor recognize TODO as a key word but nothing is done with it ?
I put //TODO or /* TODO CIO-SLI ESSAI TASKVIEW */ then i Launch "rebuild index". I close and open the task view but nothing appears in it.

I have read this bug page with a google search.I tought I just have to update the plugin with eclipse software update but  infortunately this patch is not included (cdt version 3.1.2.20070215062).

Is there a CDT version compatible with eclipse 3.2.2 that support it ? Or do i have tu update to eclipse 3.3 + cdt 4.0 .

Many thanks !
Comment 50 Markus Schorn CLA 2007-06-25 05:07:21 EDT
(In reply to comment #49)
> Is there a CDT version compatible with eclipse 3.2.2 that support it ? Or do i
> have tu update to eclipse 3.3 + cdt 4.0 .

As the target milestone of this bug indicates, you need to use CDT 4.0.