Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 286294

Summary: support starring of comments
Product: z_Archived Reporter: Mik Kersten <mik.kersten>
Component: MylynAssignee: Project Inbox <mylyn-triaged>
Status: CLOSED MOVED QA Contact:
Severity: enhancement    
Priority: P3 CC: lucas.panjer, mik.kersten, peter, sam.davis, steffen.pingel, tomasz.zarna
Version: unspecifiedKeywords: plan
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard: effort=3;
Bug Depends on: 358554    
Bug Blocks: 168363, 343600    
Attachments:
Description Flags
quick and dirty implementation of private starred comments
none
mylyn/context/zip
none
patch for master none

Description Mik Kersten CLA 2009-08-11 13:16:41 EDT
Some comments are more interesting than others.  For example, a comment can contain relevant documentation, feedback, or a specification.  We can make these stand out by adding a starring mechanism to comments, which causes any starred comment to remain expanded between task editor open.
Comment 1 Steffen Pingel CLA 2009-08-11 13:39:28 EDT
How and where would the information about staring be persisted?
Comment 2 Mik Kersten CLA 2009-08-11 18:58:13 EDT
It seems like user-specific information to me, since something that's interesting to me may not be interesting to you.  Should it be kept in the Task Data?
Comment 3 Steffen Pingel CLA 2009-08-11 19:27:48 EDT
Task data is managed by the connectors and contains repository information and we don't have a good way to add other local information at the moment. It seems that staring is a connector independent framework concern similar to the personal notes. We could consider adding a memento like facility to hold addtional data for tasks that is stored somewhere in the .mylyn folder.
Comment 4 Peter Stibrany CLA 2009-08-17 13:15:10 EDT
FogBugz repository has concept of starring the case. It would be nice if "starred task" information can be stored in/retrieved from the repository too. (Unfortunately FogBugz exposes no API for this feature at the moment :-(, but may do so in the future).
Comment 5 Mik Kersten CLA 2009-10-01 14:25:50 EDT
Need to move to backlog unless someone wants to contribute.
Comment 6 Mik Kersten CLA 2010-07-02 01:26:24 EDT
Raising priority due to the high value and low effort nature of this.
Comment 7 Mik Kersten CLA 2011-02-18 16:02:06 EST
Steffen: I assume this can't make 3.5 so adding to 3.6 plan.
Comment 8 Steffen Pingel CLA 2011-02-18 16:17:17 EST
Yes, unfortunately we are already overloaded for 3.5 so 3.6 would be the earliest to do this.
Comment 9 Steffen Pingel CLA 2011-07-28 03:16:58 EDT
*** Bug 353263 has been marked as a duplicate of this bug. ***
Comment 10 Sam Davis CLA 2011-07-28 14:28:17 EDT
I would like the starred state to be stored on the repository and shared, so that if one person stars a comment, everyone sees it. I suppose there might also be value in having private stars but having both would probably be too complicated. But I think generally if a comment is important, it is important to everyone.

From Bug 353263:

bq. It would be great to have some way of starring important comments on a task. This state would be stored in the repository so that it is shared between all users. As an example use case, sometimes someone will post a comment saying something like "I'll leave this open for now in case there is any more feedback, but if not then we are done here." When that person (or someone else) later returns to that task, it would be helpful to have that comment stand out somehow so that they know they don't have to read any earlier comments to determine the state of the task. Perhaps this could be supported by setting some metadata on the task attribute.

I guess this specific use case could be addressed by using the status whiteboard in Bugzilla, but then I would want it to be in a more primary location. Using comments just seems more natural anyway.
Comment 11 Mik Kersten CLA 2011-09-07 18:55:04 EDT
Sam: I can see the use cases for that feature.  However, if comment starring is stored in the repository, I think it needs to be be supported by that repository.  Otherwise what we're doing is layering on features that feel like they are part of the repository's data model, whereas they are not.  In contrast, we have a very clear set of cross-repository features that are private and specific to the user, such as read state, scheduling, and drafting comments.  This feature request is intended to add to that.  Just as I want to be able to flag an email as important, I want to be able to flag a comment as important.  Could you raise a separate request for supporting repository-stored starring and CC me?
Comment 12 Sam Davis CLA 2011-09-07 19:57:41 EDT
Mik, I did (bug 353263), but it was marked as a duplicate of this.
Comment 13 Sam Davis CLA 2011-09-20 21:27:32 EDT
Created attachment 203718 [details]
quick and dirty implementation of private starred comments

This is suitable for individual use (I did it because I am going to use it myself). Patch can be applied to workspace root by setting "ignore leading path segments" to 1.
* Needs images for starred and unstarred (currently uses checkboxes). 
* Behaviour of Expand Starred could possibly be refined.
Comment 14 Sam Davis CLA 2011-09-20 21:27:35 EDT
Created attachment 203719 [details]
mylyn/context/zip
Comment 15 Sam Davis CLA 2011-09-20 21:36:58 EDT
Created attachment 203720 [details]
patch for master

The previous patch was for the service branch, here's one that works for master.
Comment 16 Steffen Pingel CLA 2011-09-21 07:10:46 EDT
This looks like a good approach. I am wondering though if we need to keep the information about starred comments in memory or if it would be sufficient to store this on disk and load it when the task editor is opened?

Sam, this is a great feature addition. Would you be willing to encapsulate management of starred state in a class that is not directly coupled to the editor implementation add add a test case? Feel free to push changes to http://review.mylyn.org/ if you want to move this forward through a code review.
Comment 17 Sam Davis CLA 2011-09-21 11:53:30 EDT
(In reply to comment #16)
> This looks like a good approach. I am wondering though if we need to keep the
> information about starred comments in memory or if it would be sufficient to
> store this on disk and load it when the task editor is opened?

Given that the user can modify the state at any time while the editor is opened, it seems like an odd thing to store on disk. Actually, I am storing it in the task data - I don't know if that's the right thing to do, but isn't it stored on disk when the task is not opened?

> Sam, this is a great feature addition. Would you be willing to encapsulate
> management of starred state in a class that is not directly coupled to the
> editor implementation add add a test case? Feel free to push changes to
> http://review.mylyn.org/ if you want to move this forward through a code review.

I will see if I get time to do that. I may need some help or documentation about how to push changes there.
Comment 18 Steffen Pingel CLA 2011-09-21 17:25:23 EDT
(In reply to comment #17)
> (In reply to comment #16)
> > This looks like a good approach. I am wondering though if we need to keep the
> > information about starred comments in memory or if it would be sufficient to
> > store this on disk and load it when the task editor is opened?
> 
> Given that the user can modify the state at any time while the editor is opened,
> it seems like an odd thing to store on disk. Actually, I am storing it in the
> task data - I don't know if that's the right thing to do, but isn't it stored on
> disk when the task is not opened?

Sorry if I didn't explain myself well. Looking at the code it seems that the state gets stored in the task list which means its kept in memory all the time:

 getTaskEditorPage().getTask().setAttribute(COMMENT_STARRED + taskComment.getNumber(), Boolean.toString(starred))
 
I was just wondering if it makes sense to persist it to a separate file that is only loaded into memory when the editor is opened. We have the same problem with notes, task activity and other data and this would be a good driver to create a generic facility to persist task related data that is not required to present the task list.
Comment 19 Sam Davis CLA 2011-09-21 19:39:43 EDT
Oh, I didn't even realise that was storing it in the task list. I think having a separate store would be a good idea, but it seems like a separate task. Would we want anything more than just (String, String) key-value pairs?
Comment 20 Sam Davis CLA 2011-09-21 19:40:26 EDT
I suppose anything that exports the task list might need to be updated to also export this new store.
Comment 21 Steffen Pingel CLA 2011-11-23 05:11:36 EST
*** Bug 363984 has been marked as a duplicate of this bug. ***
Comment 22 Sam Davis CLA 2012-01-04 17:09:27 EST
I have created a review at http://review.mylyn.org/181
Comment 23 Sam Davis CLA 2012-03-15 12:41:31 EDT
It would be interesting to have some way of saying things like, "If you read comments 12-13, you can probably ignore comments 5-11," i.e. some way of indicating that a comment or comments summarize the discussion of some previous comments. That could make it much easier to get up to speed on tasks with a large number of comments. It would be sort of like post facto comment threading.
Comment 24 Mik Kersten CLA 2012-03-15 17:38:03 EDT
Seems like fancy hyperlinking could do the trick for that, eg, comments 12-13 would be hyperlinked and cause the expansion state of that comment range to toggle.

For now I would just be happy with starring :)
Comment 25 Sam Davis CLA 2012-05-15 14:15:44 EDT
What is the status of this? I see that the review has been abandoned.
Comment 26 Steffen Pingel CLA 2012-05-15 15:57:00 EDT
It has just been moved to Eclipse.org: https://git.eclipse.org/r/#/c/5634/.
Comment 27 Mik Kersten CLA 2012-05-28 18:51:41 EDT
Steffen: Are you pushing ahead with this?  Let me know if we should do a paired session on it.
Comment 28 Steffen Pingel CLA 2012-06-03 17:54:39 EDT
The bug has a high priority on the backlog but we are not considering it for 3.8 due to time constraints.
Comment 29 Eclipse Webmaster CLA 2022-11-15 11:45:08 EST
Mylyn has been restructured, and our issue tracking has moved to GitHub [1].

We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub.

[1] https://github.com/orgs/eclipse-mylyn