Bug 178474

Comment 1 Mik Kersten 2007-03-21 21:12:16 EDT

Tentatively scheduling for M2.

Comment 2 Robert Elves 2008-06-14 01:10:44 EDT

Need to defer: http://wiki.eclipse.org/index.php/Mylyn/3.0_Plan#Deferred_Items

Comment 3 Mik Kersten 2008-06-16 12:25:40 EDT

I think that this provides a big bang for the buck, so tentatively raising priority for 3.1.

Comment 4 Steffen Pingel 2009-08-05 20:12:03 EDT

In terms of API I am thinking that it could make sense to provide an extensible class similar to IMarker which allows setting of custom attributes. An instance of this would be passed to the editor page such as a comment id, attachment id or attribute id and the page would make the corresponding UI element visible.

Comment 5 Mik Kersten 2009-08-06 12:54:49 EDT

I wonder if we coudl have simle API for select/reveal which takes an IPath of string values?  Eg, 
* [ Comments, 4 ]
* [Attributes, Severity]

Comment 6 Steffen Pingel 2009-08-07 18:36:31 EDT

I don't think it's a good idea to include the path since that is specific to the layout of the editor whereas most clients of the API will operate on the model (the selection listener in AbstractTaskEditorPage.updateOutlinePage() provides that functionality already). 

I can see the following model elements that would be useful to support for select/reveal:

 * attribute (by id)
 * comment (by number, id)
 * attachment (by name?)

 

Comment 7 Frank Becker 2009-11-25 15:34:34 EST

Created attachment 153117 [details]
POC

This is a POC that use the patch of bug# 250257 to implement support of attachments in the outline view. If you select an attachment the row in the table is highlighted.

Comment 8 Frank Becker 2009-11-25 15:34:38 EST

Created attachment 153118 [details]
mylyn/context/zip

Comment 9 Steffen Pingel 2010-02-28 21:09:54 EST

Changes for bug 301756 have been completed which provides basic infrastructure for revealing elements in the task editor. 

To resolve this bug a listener would need to be registered in OpenCorrespondingTaskAction that invokes selectReveal() on the editor when the task is opened. The listener would need to pass the attribute id of the comment that is to be revealed. 

Not sure what the priority of implementing this is. Moving to the backlog.

Comment 10 Frank Becker 2010-03-05 15:09:17 EST

>This should be very straightforward to implement: we get the time from the history view entry, and we make the task editor scroll to the
>corresponding comment. If scrolling it is too hard it can just auto-expand for now.

I did not know how the mapping between the commit time and a comment time can work. I think that we get never an match. 

What we can implement is that we try to open the comment that is next after the commit time.

Thoughts?

Comment 11 Frank Becker 2010-03-05 15:12:02 EST

Created attachment 161181 [details]
mylyn/context/zip

Comment 12 Steffen Pingel 2010-03-05 16:14:54 EST

Yes, showing the comment posted after a commit was made sounds like the best approximation.

Comment 13 Frank Becker 2010-03-26 15:11:12 EDT

Created attachment 163114 [details]
patch

Here a patch.

Keep in mind that the time in the cvs viev show no seconds so you can not see if the selected comment is the right one.

Comment 14 Frank Becker 2010-03-26 15:11:21 EDT

Created attachment 163115 [details]
mylyn/context/zip

Comment 15 Steffen Pingel 2010-05-17 14:39:11 EDT

Patch looks good to me. I am wondering though if using Date instead of long as the type for the time stamp would make the API more descriptive?

One thing that needs to be changed is that we need to keep the TasksUiUtil.openTask(String repositoryUrl, String taskId, String fullUrl) method and delegate to the new method with the time stamp parameter since the method is API.

Comment 16 Frank Becker 2010-05-17 15:58:53 EDT

(In reply to comment #15)
> Patch looks good to me. I am wondering though if using Date instead of long as
> the type for the time stamp would make the API more descriptive?
Do you mean that I change long to java.sql.Timestamp?
> 
> One thing that needs to be changed is that we need to keep the
> TasksUiUtil.openTask(String repositoryUrl, String taskId, String fullUrl) method
> and delegate to the new method with the time stamp parameter since the method is
> API.
Changed to public static boolean openTask(String repositoryUrl, String taskId, String fullUrl, long timestamp)

Comment 17 Steffen Pingel 2010-05-17 18:31:56 EDT

I was thinking of java.util.Date instead of using long but long has the advantage of being immutable so it could be the better choice.

Comment 18 Frank Becker 2010-05-17 23:41:14 EDT

Created attachment 168854 [details]
commited patch

API change from comment#15 is included

Comment 19 Frank Becker 2010-05-17 23:41:18 EDT

Created attachment 168855 [details]
mylyn/context/zip

Comment 20 Frank Becker 2010-05-17 23:42:22 EDT

patch is now in HEAD

Comment 21 Steffen Pingel 2010-05-18 13:40:49 EDT

Thanks! Frank, please make sure to add your name to the copyright header of all files you modified and an @author tag to all files where you made significant changes.