[MonoDevelop] New Task List patch - preview

[David Makovský (Yakeen)]

Lluis Sanchez píše v Út 05. 09. 2006 v 21:22 +0200:
> El mar, 05-09-2006 a las 21:08 +0200, David Makovský (Yakeen) escribió:
> > Lluis Sanchez píše v Út 05. 09. 2006 v 18:01 +0200:
> > > El mié, 30-08-2006 a las 21:01 +0200, David Makovský (Yakeen) escribió:
> > > > Hi all,
> > > > 
> > > > after some time I worked again on new Task List implementation, here is
> > > > preview patch. I would appreciate comments on code, gui and
> > > > functionality.
> > > > 
> > > > Basic description what is done:
> > > > 
> > > > There is new Task List option in preferences where you can setup tokens
> > > > for aggregating tasks from comments (eg. //TODO: do something) and
> > > > Choosing colors for priorities highlighting.
> > > > 
> > > > Next we have new Task List pad ("old" one is renamed to "Errors List")
> > > > and this one have an toolbar with combo to switch between "generated"
> > > > tasks and user tasks.
> > > > 
> > > > User Tasks are interactively editable, you can set three values -
> > > > priority, completion, description. Priority causes color highlighting of
> > > > tasks. These tasks are stored in userprefs file of main solution.
> > > > 
> > > > Generated tasks are filled when files are parsed (this patch ads support
> > > > for C#, as I know VB has this, other languages are unknown to me, but
> > > > filling ICompilationUnit.TagComments when parsing will do the trick...).
> > > > Collected data are stored in pidb files together with last valid Tokens
> > > > that was used for parsing. Hopefully all situations when you change your
> > > > tokens are handled well, I tried to prevent unnecessary re-parsing, but
> > > > for first time you have to :)
> > > > Generated Tasks have no priority highlighting yet, but it is on my TODO
> > > > for future.
> > > > 
> > > > So let me know what is your experience.
> > 
> > 
> > Hi Lluis,
> > firstly thanks for feedback, exactly in directions I expected :) 
> > 
> > > Some comments:
> > > 
> > > I think it would be better to store user tasks in its own file, instead
> > > of the .userprefs file. This file stores user preferences for the
> > > combine, and tasks are not preferences, but real data.
> > 
> > When I think about, it has sense. I'll do it this way.
> > 
> > > The decision about whether the a parser db has to be reparsed or not has
> > > to be taken by the db itself, not by the GUI. The code completion
> > > database should take care of monitoring changes in the
> > > "Monodevelop.TaskListTokens" property, and reparse the db when needed.
> > > The methods LastValidTaskListTokens, RemoveSpecialCommentTag and
> > > QueueAllFilesForParse would not be needed in this way.
> > 
> > I thought it would need re-factoring, I had bad feeling about exposing
> > some functionality to world.
> > 
> > > I'm also wondering if we should have a more generic and plugable task
> > > service, so we can integrate other task sources, such as bugzilla or
> > > sourceforge. But I still don't know what would be the best way of
> > > modeling this.
> > 
> > I'm also not sure how to do it exactly, I was minding about abstracting
> > view little bit to expose functionality to design and work with data.
> > Then we can have plug-able content views. I'll do some experiments on
> > this.
> > 
> > > Comments on the code:
> > > 
> > > DefaultParserService.cs:1093
> > > The call to OnParseInformationChanged should not be necessary since it
> > > is already called by DoParseFile().
> > 
> > hmm, sure I see now
> > 
> > > DefaultParserService.cs:1610
> > > Why update coreDatabase?
> > 
> > hmm, there is nothing like this around this line, may you specify a
> > function name?
> 
> Sorry, that's DefaultParserService.cs:1610
> 
> @@ -1610,6 +1632,7 @@
>  
>  		protected virtual void
> OnParseInformationChanged(ParseInformationEventArgs e)
>  		{
> +			this.coreDatabase.UpdateTagComments
> ((((ICompilationUnit)e.ParseInformation.ValidCompilationUnit).TagComments), e.FileName);
>  			if (ParseInformationChanged != null) {
>  				ParseInformationChanged(this, e);
>  			}
> 
> Lluis.
> 

Yeah, that was for updating db after re-parsing, that's nonsense, this
would be done by db itself...

David.