This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 145635 - Support for debug view pin & clone
Summary: Support for debug view pin & clone
Status: CLOSED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
: 372354 (view as bug list)
Depends on:
Blocks: 398012 372354
  Show dependency tree
 
Reported: 2006-06-06 16:50 EDT by Darin Wright CLA
Modified: 2019-11-27 07:18 EST (History)
23 users (show)

See Also:


Attachments
patch (54.29 KB, patch)
2006-06-06 16:57 EDT, Darin Wright CLA
no flags Details | Diff
Patch adding Pin+Clone feature to Variables View (116.40 KB, patch)
2007-01-03 15:05 EST, Pawel Piech CLA
no flags Details | Diff
Patch adding the "Pin+Clone" feature to Variables and Memory views, based on head branch as of Jan 26 '07. (167.74 KB, patch)
2007-01-26 16:50 EST, Pawel Piech CLA
no flags Details | Diff
Icon: org.eclipse.debug.ui\icons\full\obj16\window_context.gif (935 bytes, image/gif)
2007-01-26 16:52 EST, Pawel Piech CLA
no flags Details
Icon: org.eclipse.debug.ui\icons\full\elcl16\context_scope.gif (105 bytes, image/gif)
2007-01-26 16:53 EST, Pawel Piech CLA
no flags Details
Plugin that creates a "Red" debug view, for testing. (9.28 KB, application/zip)
2007-01-26 20:08 EST, Pawel Piech CLA
no flags Details
Presentation with diagrams of different debug context tracking configurations. (262.50 KB, application/octet-stream)
2007-01-26 20:49 EST, Pawel Piech CLA
no flags Details
Plugin containing scope interfaces and implementation. (128.05 KB, application/x-zip-compressed)
2007-04-24 13:21 EDT, Pawel Piech CLA
no flags Details
Patch to the UI plugin to enable the scope plugin, based on M6. (14.33 KB, patch)
2007-04-24 13:24 EDT, Pawel Piech CLA
no flags Details | Diff
Updated patch to work with Eclipse R3_3 (37.61 KB, patch)
2009-09-22 13:56 EDT, Alain Lee CLA
no flags Details | Diff
Update debug.ui.scope plugin to work Eclipse R3_3 (126.69 KB, patch)
2009-09-22 13:57 EDT, Alain Lee CLA
no flags Details | Diff
Updated org.eclipse.debug.ui patch to work with Eclipse R3_5 (39.32 KB, patch)
2009-09-28 15:50 EDT, Alain Lee CLA
no flags Details | Diff
updated org.eclipse.debug.ui.scope plugin to work with Eclipse R3_5 (126.86 KB, application/x-zip-compressed)
2009-09-28 15:52 EDT, Alain Lee CLA
no flags Details
Breadcrumb in variables view patch. (65.73 KB, patch)
2012-08-24 11:48 EDT, Pawel Piech CLA
no flags Details | Diff
Breadcrumb in variables view patch (updated). (53.43 KB, patch)
2012-08-24 13:37 EDT, Pawel Piech CLA
no flags Details | Diff
Patch for JDT with memento providers for Debug view. (5.67 KB, patch)
2013-01-11 19:07 EST, Pawel Piech CLA
no flags Details | Diff
Updated patch for JDT. (5.55 KB, patch)
2013-01-14 12:29 EST, Pawel Piech CLA
no flags Details | Diff
Screenshot of pin drop-down. (12.83 KB, image/png)
2013-01-23 00:48 EST, Pawel Piech CLA
no flags Details
empty var view after step (313.39 KB, image/png)
2013-01-23 11:02 EST, Michael Rennie CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Wright CLA 2006-06-06 16:50:08 EDT
Some debuggers require the ability to tile views side by side for visual comparison. For example, one may want to debug a client and server at the same time with a variable view for each process visible at the same time. Or, one may want to debug two versions of the same application with a variables or registers view from each application visible at the same time for comparison. This article proposes support that will allow debug views to be cloned and pinned to a specific debug context - i.e. pin one variables view to a client, and another to a server, and view them side by side.

The 3.2 debug platform introduced provisional API for listening to and driving the debug context. The views and actions in the platform listen to the active debug context in their associated window such that any active debug context provider will drive the actions and views making them retargettable. In 3.2, the only context provider provided by the platform is the debug view, which provides the active context based on selection in the debug view. When providing support for pinning a view to a specific context, we still want to maintain retargetting support. For example, if there are two different views or editors capable of driving the debug context for a debug session (say the debug view and a disassembly view), then as a user moves between the views, a variables view that is pinned to that debug context should respond to changes in that context no matter which view the change originated from.
Comment 1 Darin Wright CLA 2006-06-06 16:51:31 EDT
Implemented first cut of 'scoped context' infrastructure that can be used to support pin & clone. Will attach as patch, with tests.
Comment 2 Darin Wright CLA 2006-06-06 16:57:38 EDT
Created attachment 43651 [details]
patch
Comment 3 Darin Wright CLA 2006-09-25 10:07:01 EDT
Not committed for 3.3
Comment 4 Pawel Piech CLA 2007-01-03 15:05:17 EST
Created attachment 56347 [details]
Patch adding Pin+Clone feature to Variables View

The patch includes the following:
- Made changes to the IDebugContextManager to allow multiple debug context providers with no associated part-id to be added.
- Simplified up IDebugContextService interface, and made it so that each debugger view has its own instance of IDebugContextService.  Actions which are registered with a view, listen to the context service from the view, and actions which are registered with the global menu, listen to the context service belonging to the window.
- Added an implementation of IDebugContextProvider which "filters" a selection coming from a source context provider (such as debug view), so that only a sub-tree of the original provider is active.  Thus if a view is "pinned" to a thread, the view will display any stack frame that is selected within that thread.
- Added actions to variables view to create sub-tree IDebugContextProviders based on current selection in window, and to "pin" the view to one of the sub-tree providers.
- Added logic to clone the variables view.

Because of the ongoing changes in related plugins, I created this patch against the v20061212 label, which roughly corresponds to the M5 milestone.
Comment 5 Pawel Piech CLA 2007-01-11 14:17:50 EST
In a week or so, I plan to update the patch to work with HEAD version of platform and to make following changes:

- Add a banner at the top of the view when the view is pinned, banner will change background color based on IColorProvider associated with the IDebugContextProvider
1) Change the behavior so that the global actions do not work in context of the pinned view, but always respond to the active selection in the current window.
2) Update the view label to include the first 10 characters of the IDebugContextProvider label.
3) Modify the IDebugContextManager API to allow listeners to register to listen to views based on the view ID.  So that listeners could be registered before the views are created.
Comment 6 Boris Bokowski CLA 2007-01-17 11:30:38 EST
See also bug 169581.
Comment 7 Pawel Piech CLA 2007-01-26 16:50:51 EST
Created attachment 57628 [details]
Patch adding the "Pin+Clone" feature to Variables and Memory views, based on head branch as of Jan 26 '07.
Comment 8 Pawel Piech CLA 2007-01-26 16:52:45 EST
Created attachment 57629 [details]
Icon: org.eclipse.debug.ui\icons\full\obj16\window_context.gif
Comment 9 Pawel Piech CLA 2007-01-26 16:53:53 EST
Created attachment 57630 [details]
Icon: org.eclipse.debug.ui\icons\full\elcl16\context_scope.gif
Comment 10 Pawel Piech CLA 2007-01-26 20:06:55 EST
I attached a patch for this feature based on version of sources in CVS.  The patch includes the following changes:

1) I backed off from using the "Pin" name and icon.  Darin mentioned that this might be over-use of this term, and come to think of it, the feature we are trying to implement is not really "Pin".  To me pin means to freeze the view from making any updates, but what we are doing is only restricting which context changes will cause an update in the view.  In trying to match this idea, I renamed the pull-down action to "Set View Context Scope".  I really don't know how meaningful this might be to everyone, especially at first but IMO it more accurately describes the action.  I also created new icon for the action, which I'm not entirely crazy about, but I had to make it in a hurry.  P.S.  The tipping point for renaming the action came when I saw that the memory view already has a "Pin" action :-(

2) I added the "set view context scope" action to memory view, although I had to duplicate some 200 loc, as there was no common place to put it.  I also noticed that memory view actions use DebugUITools.getDebugContext() to retrieve the current debug context in the view.  These actions should probably be reviewed.

3) Architecturally, I implemented the "scopes" as additional debug context providers, and made each window and view have its own debug context service.  I will create and attach a short document with diagrams of the changed debug context framework.

4) Visual indicators:  
 In both Variables views and Memory view, I settled on using the IViewPart.setContentDescription() and IViewPart.setTitleTooltip() to show the name of the context provider that the view is listening to, as well as the current active context.  This content description is only visible if the views are scoped to a context provider other than "Window".  Unfortunately, this makes for long and difficult to read strings to appear in the content description area, but I don't know if there's any way to address this.  In cases where the view is not on top of the stack, the tooltip provides a quicker method for identifying which view instance is which.
 In the variables view I also included an experiment for drawing a thin color border around the view, when the view is scoped to a provider that returns a color.  The attached example "Red Debug" view provides such a color.  

5) Clone action is a separate action on the tool-bar.  Memory view already had this action, so I followed the same pattern in variables views.

Known issues:
a) The "sub-tree" context providers, which are generated on the fly based on window selection, have no way of knowing when an element has been removed.  So if the user selected to restrict the context scope to a thread, and that thread is removed, the view will continue to try to evaluate against this stale thread object.  I think the only way to address this, would be to require the original provider to send DebugContextEvent.CONTENT events which would correspond to the IModelDelta.CONTENT updates, and for the sub-tree context provider to actively retrieve children of the objects that it's pinned to, using flexible hierarchy IElementContentProvider interface.  The alternative is to live with the behavior as it is now.

b) The experimental color border visual indicator in variables view is painted within the default page of the variables view.  When a different view page, such as message is brought to front, the color border disappears.  To address this, I would like to get rid of the color border, and instead color the view icon using the foreground and background colors associated with the selected context provider.  I think painting the icon would also give a good color-cue indicator to the user, which is in addition to the view's "content description".

c) Persistence of view state.  When view context scope is configured to point at a given provider, such as the Debug view, it would be ideal if that fact was preserved between Eclipse sessions.  I planned to implement this feature by requesting the IPersistableElement/IElementFactory from the context provider.  This may take a lot of work to implement by the providers but I don't believe that there are any insurmountable technical challenges there.

d) SetContextScopeAction calls the ILabelProvider methods on the UI thread.  This is a problem because internally, some of the providers call the asynchronous IElementLabelProvider update in order to obtain the label.  I've been meaning to fix this for a while, but I ran out of time.  BTW, the variables views and the memory view call the label provider using a job.

Further extensions:
Beyond addressing issues (b) and (c) above, I plan to implement a mechanism for allowing the user to configure a set of custom context scopes.  These scopes would not need to be associated with any view and could be identified using a user-specified label, icon overlay, and color.  To be useful, these contexts would also need to affect the presentation of model elements in all views, including breakpoints and instruction pointers, and would need to be persisted across debug sessions.  I do not believe I will be able to complete this work in time for 3.3 release, but probably soon thereafter.
Comment 11 Pawel Piech CLA 2007-01-26 20:08:30 EST
Created attachment 57637 [details]
Plugin that creates a "Red" debug view, for testing.
Comment 12 Pawel Piech CLA 2007-01-26 20:49:43 EST
Created attachment 57638 [details]
Presentation with diagrams of different debug context tracking configurations.
Comment 13 Samantha Chan CLA 2007-01-30 00:27:50 EST
Hi Pawel,

I have tried the patch... and here are some of my feedbacks:

Re:  Visual Indicator
I found the text on the banner a bit confusing when the view is pinned.  

For example, when the view is pinned to the Debug view, the banner states: Debug - <stackfame label>.  
At first, I thought the view was pinned to the stackframe, not aware that "Debug" means the debug context provider.

Another example is when the view is pinned to a stackframe, the banner states:
<stacframe label> - <stackframe label>.  I thought the label was repeated unintentionally.

I understand that you are trying to give the user an idea of what the view is currently pinned to. 

So, maybe the banner text can be constructed as follows:
[Context Provider]: Debug  [Current Context]: <stackframe label>

The problem with this is that we will end up with an even longer string. :(

Re: Use of DebugUITools.getDebugContext in the Memory View's actions
You are right.  All of those actions need reviewing.  I think this affects more than just the Memory View.  There are JDT actions (like content assistance) that makes use of the current debug context.  We have to review all its usage and make sure that the actions are not broken when the view is pinned.

I think this is a migration issue in general.  This affects clients who provide
preferences / actions to debug views.  Clients need to be aware of these changes and make appropriate updates.

Re:  Persistence of view preferences
By view preferences, I mean the column settings, view layouts and some of the JDT-specific preferences that are contributed to the Variables View.  Many of these settings are stored globally.  We need to review all of these preferences and make sure that they are not broken when the view is cloned.

An example of this is the horizontal/vertical layout setting in the Variables View.  If I have two variables view opened and changed one view to have the veritical layout.  The view will be switched to the veritical layout, while the other stays the same.  However, when I restart the workbench, both views will have the layout last set on the view.

Re:  Set View Context Scope Action
Do you think it is a good idea to provide an action that pins to the current
context in the view?  Most of the time, the user may be looking at the view and wants to pin it, clone and then compare the content with another context.  (Is this one of the most common use cases?)  In this case, providing a quick way to pin may be desirable.  

(It takes many clicks to get to the drop down menu and find the stackframe to pin the view to.)

In addition, it will be nice to have a quick button and shortcut that allows me to unpin and refresh the view to the current debug context.

Re:  Pinning action in the Memory View
This action can be renamed if needed.  In addition, I think this action needs to be reviewed.  This action is used to pin the Rendering panes to a memory block and prevent the view from switching "memory block context" when a new memory block is added from another view.  

Ideally, the Memory Monitors pane should listen for debug
context events from the view's debug context service.  In turn, the rendering panes should listen for debug context events from the monitors pane.  i.e. each of the memory monitor becomes a debug context provider.  I tried following this when implementing the rendering view pane... but I may not have cleaned it up completely.

Re:  Show info from stale debug elements
The current behavior is that if the thread / stackframe becomes invalid, the view stays the same, showing data from the stale debug elements.  I am wondering if it may be a good idea to some how show the view as disabled if this is the case?  This gives the user an idea that they are not looking at live data.

In addition, what should happen when the user a variable/register from a stale
stackframe?  I think I get a disconnect exception displayed if I am debugging Java.  This should be avoided.  The model should handle this and make sure that it returns no children when the stackframe / thread is stale.  In fact, this affects all models since they can get asked for things long after the model objects are no longer valid.  Some models may not handle this very well.

In the memory view, the rendering panes are cleared when the debug session is terminated if the view is pinned.  However, the monitors pane stays populated.  We should probably investigate this and make sure the rendering panes are only handling the terminate events from the "pinned" context.

Re:  Switch Memory Block Action
This is the action from the pull down menu from the Memory View. It lists
all the memory monitors that are currently available and allows the user
to switch to the selected memory monitor without going to the monitors pane.  
The memory monitors list generated for this drop down menu does not reflect 
what is currently pinned. It shows all available memory monitors based on 
the current debug context from the Debug View, not from the view's debug context service.

Re: DebugContextManager#addContextService
Each view hs a context service.  When the view is created, the view registers
the service with the debug context manager.  The manager stores the
context service in a hashtable, using the view's id and its secondary
id as the key.  

The problem here is that this will break if there are multiple
workbench windows.  If we have two workbench windows, the views on the second
window may not get a secondary id.  (Those that get opened by Windows -> Show View will only have a view id, no secondary id.)

In this case, the debug context manager will accidentally overwrite 
a debug context service from another window.  The id needs to somehow incorporate the concept of a "workbench window" to uniquely identify the view and its debug context service.  Or, each workbench window should have its own debug context manager.

Is this understanding correct?

Re:  SubTreeDebugContextProvider
The providers and actions for setting the debug context scope to a "subtree element" are contructed by getting the active debug context from the window, and going up to all its parents.

This is ok if there is only one debug context provider in the window.  (i.e. one debug view) When we have more than one debug context provider (more than one Debug view), it becomes confusing how this list is generated.  Most users are not aware of which of the views is having focus, therefore, they do not know
which one of the Debug views is the active debug context provider.

In addition, if the two debug context providers actually have different elements displayed, how does the user go from one debug context provider to another.  e.g. if I have two Debug views, one with debug session a, and one with debug session b.  How can the user pin the Variables View to a context from debug session b if the current active context provider is only showing debug session a?  

Re:  View Management
The debug platform allows clients to define views that get automatically opened
when a debug context is activated (identified by model id).  (e.g. when debugging java, we can have the expressions view opened automatically.)  When a debug context is no longer valid, the debug platform closes the views that were opened automatically.

When a view is cloned, the view is considered to be opened manually by the user.
As a result, the view is no longer under the debug platform's management and does not get closed when a debug context goes away.

This was looked at when implementing clonable Memory View and it
was decided that the behavior was acceptable.  However, since more views
can be cloned and I assume that the features will get used more, I am wondering if we should review the current behavior.  In the long run, as more views get cloned and not closed, the workbench can get clustered.  

Re:  Expressions View
The Expressions View has actions for cloning, but the view does not allow
multiples.  Is this intentional? 

Re:  DebugPluginImages
I think the patch accidentally removes the declaration of this image: IMG_ELCL_STANDARD_ERR.

Thanks...
Samantha
Comment 14 Pawel Piech CLA 2007-01-30 13:53:50 EST
Thank you Samantha for the copious feedback.  I'll try to answer your questions the best I can:

Re: Visual Indicator
  I agree that the text is rather confusing, even after seeing it for the first time.  In the patch, I decided to leave out any explanatory additions, because that's what the console view does, but I don't have a problem with making it a little longer with your suggested changes.  

Re: getDebugUIContext()
  After submitting the patch, I thought it would be good to add a DebugUITools.getDebugUIContext(String viewId, String secondaryId), which may make it easier for some clients to adapt to the changes. 

Re: Persistence of view preferences
  I didn't realize that I was breaking this.  I will have to check on how to fix it.

Re: Set View Context Scope Action
  Unfortunately "Set View Context Scope" is not nearly as familiar to users as "Pin" :-(, but I was getting the impression that platform was worried about abusing the term "Pin" so I thought it would be safer to avoid it.  

Possible compromise:  We could move the complex pull-down menu from the toolbar and into the view menu.  And instead put on the menu a simple push-style "Pin" action, which uses the same mechanism (creates a context provider) that is simply fixed to the current selection.  This would keep it simple of the 90% use case, and still allow for cases where the user has an alternative debug view, or defines a context scope for a set of cores as we talked about at the DD meeting.  We could even defer adding the pull-down menu until after 3.3, when I'll have a chance to implement the "colored" configurable context providers.

Re: Pinning action in the Memory View
I think it makes a lot of sense to experiment with using a debug context provider locally within the view.  But I'm afraid I don't know enough about the memory view to make any concrete suggestions.

Re: Show info from stale debug elements
This is probably the most crippling problem with the pin-n-clone feature as it is now.  To fix it, I think we would need to add a new flag to the DebugContextEvent, and have the launch view issue events whenever content of a node is changed.  Otherwise, the pinned provider has no way of knowing when a context has become "invalid".  I would like to try this approach, but I'm afraid it will be way too late for M5.

Re: DebugContextManager#addContextService
You are correct.  This is a bug.

Re: SubTreeDebugContextProvider
This requires some explanation.  
Problem 1: When I talked with various people about the Pin feature, some people felt that pinning to a stack frame alone is not always very useful, because some users are more interested in all stack frames of a given thread rather than simply a single stack frame.  
Problem 2: I was also worried that in some debug model implementations the IStackFrame.equals() does not return true for the same frame after a step operation is performed.  I know that in our debugger we did not implement the equals method on the stack frame object because it caused update issues in Debug view.  

For these two reasons, I implemented the auto-generated set of sub-tree providers each for the different level in the currently selected tree path.  I personally feel that this is probably overcomplicating things, but I was trying to be inclusive.  As I see things now the way I would address the two problems is:
Problem 1: Pinning to current context is what users want 90% of the time.  If users want something more sophisticated, we should use provider a way to create a configurable context scope identified by a unique color/symbol, etc.  And this is what many people at the DD meeting were asking for.
Problem 2: Either leave it as is which means that for some debuggers after stepping, thew views will have to be re-pinned.  Or try to use IElementMementoProvider to compare elements, rather than just using equals().

Re:  View Management
I didn't even realize this feature existed and I'm not sure if it would need to be addressed for the clone use-case.

Re: Expression View
Oops, this is bug in the patch.

Re: DebugPluginImages
Also a bug.


Thanks again for the feedback.  I'm not sure what to do at this point.  I would like to update the patch to at least fix the bugs if it might be contributed in M5.  But I also have some commercial work to do that is becoming increasingly urgent and I will need to take a personal leave of a few weeks fairly soon.
Comment 15 Darin Wright CLA 2007-02-07 10:49:34 EST
We have reviewed the contribution, but believe that the implementation could be provided in the form of a 'scope filter' rather than using dedicated 'context providers' to provide context information for a scope.

The approach of using a 'scope filter' on a view to filter incoming context changes allows any context provider to drive the interactions. Note that we still would need a way to determine the current context in a scope to clone a part to an existing scope (which a context provider does have).

The current implementation of debug contexts is modeled after the selection service in the workbench.

Currently, we do not plan to commit this feature to the 3.3 HEAD stream, as we do not have sufficient time/resources.
Comment 16 Darin Wright CLA 2007-02-07 17:43:28 EST
marking as later, for consideration in the next release.
Comment 17 Pawel Piech CLA 2007-04-24 13:21:14 EDT
Created attachment 64768 [details]
Plugin containing scope interfaces and implementation.

This is work in progress on the pin and clone functionality.  I started implementing it as a separate plugin only because we plan to use this functionality before it will have a chance to be integrated into the UI plugin.  However this plugin requires a patched version of the org.eclipse.debug.ui plugin, which will be attached as well.

Functionality so far:
- The scope plugin supplies a second variables view which has a "pin" action on the tool bar.  
- Once the view is pinned it does not track the selection in Debug view.
- If the element that is pinned is removed from the debug view, the variables view un-pins itself from the stale element.
- If the element that is pinned in Debug view is removed, and another element is inserted which matches the old element through Object.equals(), the pinned view automatically switches to the new element.
- Also if the pinned element is removed, and another element with the same memento is inserted.  The pinned view autoamatically switches to the new element with a matching memento.
Comment 18 Pawel Piech CLA 2007-04-24 13:24:11 EDT
Created attachment 64769 [details]
Patch to the UI plugin to enable the scope plugin, based on M6.
Comment 19 James Blackburn CLA 2007-07-20 11:16:58 EDT
Just to say that this is a much wanted feature here, and Pawel's patches and contribution seem to work really well with CDT 4.

I was wondering what the Status: Resolved Later (deprecated) meant.  Does it mean that this feature request will be addressed, just not in this way, or does it mean that the entire feature is somehow deprecated?
Comment 20 Pawel Piech CLA 2007-07-20 13:07:47 EDT
I imagine that the platform guys are thinking about the big picture impact of this feature.  I've recently spent some time pondering that myself and I wrote up my ideas in a message on platform-debug-dev:
http://dev.eclipse.org/mhonarc/lists/platform-debug-dev/msg01179.html
Comment 21 Denis Roy CLA 2009-08-30 02:20:19 EDT
As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.
Comment 22 Alain Lee CLA 2009-09-22 13:56:22 EDT
Created attachment 147809 [details]
Updated patch to work with Eclipse R3_3
Comment 23 Alain Lee CLA 2009-09-22 13:57:24 EDT
Created attachment 147811 [details]
Update debug.ui.scope plugin to work Eclipse R3_3
Comment 24 Alain Lee CLA 2009-09-28 15:50:11 EDT
Created attachment 148272 [details]
Updated org.eclipse.debug.ui patch to work with Eclipse R3_5
Comment 25 Alain Lee CLA 2009-09-28 15:52:11 EDT
Created attachment 148273 [details]
updated org.eclipse.debug.ui.scope plugin to work with Eclipse R3_5
Comment 26 Patrick Chuong CLA 2010-09-08 12:44:53 EDT
Hi, I would like to have this feature supported for next eclipse release. I am wondering what needs to be done inorder to have the patches be committed? I would be able to spend cycles to clean up the patches and integrate the other debug views.
Comment 27 Darin Wright CLA 2010-09-08 15:55:39 EDT
(In reply to comment #26)
> Hi, I would like to have this feature supported for next eclipse release. I am
> wondering what needs to be done inorder to have the patches be committed? I
> would be able to spend cycles to clean up the patches and integrate the other
> debug views.

This is a bigger issue than simply getting the existing patches committed - the problem is understanding how such a feature should be integrated into the workbench (which requires guidance from the workbench/UI team).

Since this feature request was opened, the "bread crumb" control has been added to the debug view (i.e. compact, one line representation of a debug context). It was Pawel's hope that such a control could be embedded in views to control local debug contexts. That avenue also has to be explored.

This is a large, potentially de-stabalizing work item that needs agreement and a coordinated effort from debug and platform UI teams.

To begin, I would suggest writing up some workflows/user scenarios that you would like to support, and then descibe some potential UIs that support those scenarios. A wiki page would probably work best for such a document.
Comment 28 Patrick Chuong CLA 2010-09-08 16:11:59 EDT
(In reply to comment #27)

> To begin, I would suggest writing up some workflows/user scenarios that you
> would like to support, and then descibe some potential UIs that support those
> scenarios. A wiki page would probably work best for such a document.

Thank you Darin. I'll try to come up with workflows and mockup the UI with screen shots.
Comment 29 Pawel Piech CLA 2010-09-09 13:30:50 EDT
Thanks Patrick and Darin.  Yes, I would still like to see this feature fleshed out, but I don't have time to work on it.  Still, I would gladly review and commit a solution if we can agree on one :-)
Comment 30 Patrick Chuong CLA 2010-09-27 20:50:40 EDT
I have put together a wiki page on this feature. The requirements that I have on the wiki are something that I would like to see it get included, feel free to modify/add to the wiki.

http://wiki.eclipse.org/PinAndClone
Comment 31 Pawel Piech CLA 2010-09-28 00:44:37 EDT
(In reply to comment #30)
> I have put together a wiki page on this feature. The requirements that I have
> on the wiki are something that I would like to see it get included, feel free
> to modify/add to the wiki.
> 
> http://wiki.eclipse.org/PinAndClone

I created a companion discussion page on wiki: 
http://wiki.eclipse.org/Talk:PinAndClone.  There's already a lot of ink spilled on this topic and I hope that the dedicated discussion page will help us keep the conversation organized.
Comment 32 Pawel Piech CLA 2010-10-08 19:04:25 EDT
Based on the updated patch in bug 327263, you could prototype an breadcrumb embedded into the variables view.  To get there you'd need to create a VirtualTreeModelViewer which would be used to populate a BreadcrumbViewe (you could largely copy LaunchViewBreadcrumbViewer).  Then you could use this breadcrumb to drive the debug context provider for the variables view.

Having this as a working prototype could tell us a lot about the usability of this idea.
Comment 33 Pawel Piech CLA 2012-08-24 11:48:23 EDT
Created attachment 220277 [details]
Breadcrumb in variables view patch.

This is a proof of concept implementation of a pin to active context feature, which adds a debug-context breadcrumb to the variables views when they are pinned.  The breadcrumb allows the user to view and edit the pinned context.

The patch creates a new abstract class for views: AbstractDebugDataView, and the variables view is made to extend it.  The biggest problem left to solve is that the breadcrumb is specific to debug view, while (in theory) the variables views can be driven by any view, such as the visualizer view in CDT.
Comment 34 Pawel Piech CLA 2012-08-24 13:37:31 EDT
Created attachment 220285 [details]
Breadcrumb in variables view patch (updated).
Comment 35 Pawel Piech CLA 2013-01-11 18:42:51 EST
http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?h=ppiech/Bug145635&id=3663217732472086e14ec25ed673e0a19d00ff52

I finally finished the patch that I think should be good enough for M5.  The features in it include:

- Add clone and pin actions to the variables, registers, and memory views.  
- The pin action creates a breadcrumb that has the same contents as the Debug view, and it set to the active context at the time it was pinned.
- If the pinned context is removed (existed, etc.), a placeholder is added in the breadcrumb indicating what the view is supposed to be pinned to.
- When the context is re-created (session re-launched, etc.), the context is re-selected into the breadcrumb.
- The view clone action copies the viewer settings from the view that is being cloned into the one being created.
- The API allows other views (besides the Debug view) to be debug context providers and to create their own pin control (the breadcrumb is specific to Debug view).  Also, other projects can create pinnable views which can re-use the Debug view breadcrumb.

Features I'd like to add still
- Restore pinned context after IDE shutdown.
- Restore filters to breadcrumb when Debug view is not open.


Implementation notes:
- The largest volume of code changes are in refactoring of the Debug view breadcrumb.  I factored out a base class and have the two types of breadcrumbs derive from it.  I fixed a couple of bugs in the breadcrumb that I found along the way.
- I changed the breadcrumb viewer itself a bit to allow a client to change the font and color of text in items.
- In the tree model viewer, I added logic to save the state of the pinned context and to re-select it as needed.  Although this change was the most complicated logic in this patch and was rather tricky to debug, it is mostly additions and should not affect the viewer in other use cases.
- I added APIs to define a view that can be pinned as well as a debug context provider that can be pinned to.  A pinned context viewer factory is declared through an extension point, so that a view could restore its pinned context provider from a memento after IDE restart.  
- One possibly controversial change: In Memory view there already is a "pin" icon in the toolbar for the action "Pin memory pane".  With the pin view action being added to other views, I think it'll be more consistent to move this action to the menu (without the pin icon) and put the pin view icon in the view toolbar.  We'll need to review this with Samantha or someone on her team though.

This is a fairly big change so I would like to get it into M5 with a good time window for testing/API adjustments.  The changes are mostly additions so I don't believe they should have a bad impact on stability as some of the major changes from the past (such as the breakpoints view re-factoring).
Comment 36 Pawel Piech CLA 2013-01-11 18:53:07 EST
Mike, Dani, and Curtis,  Please try out the feature and look over the changes and see if you have any concerns.
Comment 37 Pawel Piech CLA 2013-01-11 19:07:42 EST
Created attachment 225529 [details]
Patch for JDT with memento providers for Debug view.

JDT needs a small addition to its memento providers in order to enable restoring of the pinned context in breadcrumb.
Comment 38 Pawel Piech CLA 2013-01-14 12:29:30 EST
Created attachment 225582 [details]
Updated patch for JDT.
Comment 39 Pawel Piech CLA 2013-01-16 23:33:53 EST
Patrick from TI gave some detailed feedback on cdt-dev (http://dev.eclipse.org/mhonarc/lists/cdt-dev/msg25354.html)  Here is the bullet list.

* When breadcrumb is small, only the last element in path is shown (usually stack frame).  User must hover over other elements to see their content.

I don't see a way around the limited space problem.  But one way to deal with it is to have the debugger be more concise in Debug view.  For example the TCF debugger does not expand the stack trace by default and select the thread on suspend instead.  

* The breadcrumb drop down alignment issues.  

There is a problem with drop down placement when breadcrumb is near bottom of the view or when toolbar is shifted in view.  I'll investigate.

* CDT's current pin behavior allows the variables view to partially follow changes in Debug view context, this pin implementation doesn't because it gives the breadcrumb control over selecting the context.

I think there's a way we can address this.  The pin API is designed to allow others to plug in a pin control, so we could extend this API a little more and let the user choose the pin provider with a drop down from the pin action.  CDT could then have a pin provider that supports legacy pin workflow.  I'll investigate making this extension.

* Breadcrumb does not remember the pinned context upon IDE restart.

This is one of my left TODO items.  I plan to address it in M6, after getting the API and initial mechanism settled.

* When cloning a view, it uses the current view’s pinned debug context by default. 

I'll change the default to never pin the cloned view.

* Find it faster to un-pin and re-pin the current context than to select current context from breadcrumb.

* CDT's Memory Browser and Disassembly view currently don't use the new pin control.

Didn't get to it yet.  I'll add it once the API is settled.
Comment 40 Patrick Chuong CLA 2013-01-17 10:57:51 EST
(In reply to comment #39)
> I don't see a way around the limited space problem.  But one way to deal
> with it is to have the debugger be more concise in Debug view.  For example
> the TCF debugger does not expand the stack trace by default and select the
> thread on suspend instead.  

If the stackframe is not selected, then variables will not be visible in the Variables view.

> I think there's a way we can address this.  The pin API is designed to allow
> others to plug in a pin control, so we could extend this API a little more
> and let the user choose the pin provider with a drop down from the pin
> action.  CDT could then have a pin provider that supports legacy pin
> workflow.  I'll investigate making this extension.

This will be a great addition! I think we should also have a default pin provider that can be customize by different product. Like how the breakpoint type work.

CDT current changes the pin icon color that matches the overlay icon declaration in the debug view. How do we handle this if the legacy CDT pin provider is selected?
Comment 41 Curtis Windatt CLA 2013-01-22 14:58:35 EST
I don't have steps to reproduce.  Just was debugging a target Eclipse instance.

eclipse.buildId=I20130115-1300
java.fullversion=JRE 1.7.0 IBM J9 2.6 Windows 7 amd64-64 20120809_118929 (JIT enabled, AOT enabled)
J9VM - R26_Java726_SR2_20120809_0948_B118929
JIT  - r11.b01_20120808_24925
GC   - R26_Java726_SR2_20120809_0948_B118929
J9CL - 20120809_118929
BootLoader constants: OS=win32, ARCH=x86_64, WS=win32, NL=en_US
Command-line arguments:  -os win32 -ws win32 -arch x86_64

Error
Tue Jan 22 13:54:40 CST 2013
Problems occurred when invoking code from plug-in: "org.eclipse.debug.ui".

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4373)
	at org.eclipse.swt.SWT.error(SWT.java:4288)
	at org.eclipse.swt.SWT.error(SWT.java:4259)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:468)
	at org.eclipse.swt.widgets.Widget.getDisplay(Widget.java:582)
	at org.eclipse.debug.internal.ui.views.launch.AbstractLaunchViewBreadcrumb.labelUpdatesComplete(AbstractLaunchViewBreadcrumb.java:604)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider$3.run(TreeModelLabelProvider.java:495)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider.notifyUpdate(TreeModelLabelProvider.java:488)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider.updateComplete(TreeModelLabelProvider.java:479)
	at org.eclipse.debug.internal.ui.viewers.model.LabelUpdate.performUpdate(LabelUpdate.java:157)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider$2.run(TreeModelLabelProvider.java:423)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:135)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4144)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3761)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1049)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:939)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:79)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:587)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:542)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:353)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:180)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:88)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
	at java.lang.reflect.Method.invoke(Method.java:613)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:629)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:584)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1443)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1419)
Comment 42 Pawel Piech CLA 2013-01-23 00:38:03 EST
(In reply to comment #40)
> This will be a great addition! I think we should also have a default pin
> provider that can be customize by different product. Like how the breakpoint
> type work.
I implemented a drop-down pin action which allows you to select the pin context factory.  I think this makes the feature a lot more powerful in that it would allow the user to pin the view to other sources of context.  For example we are discussing a use case in our product where the user would like to examine registers of a context without starting a full debug session.  The pin drop-down workflow seems perfectly suited for this purpose.  It should also be able to support the legacy CDT pin method.

I haven't yet implemented an enablement expression support for the pin context factories, or a method to select the default.  We need to settle on an API by this week so I wanted to post an early version of the pin drop-down to make sure it's on the right track.

The only down-side of the pin drop down action that I've found is that the drop-down action doesn't support a toggle state.  I.e. when view is pinned, the pin action does not show it.  OTOH, the pinned context viewer control should give a good visual indication that the view is pinned to a context.

http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?h=ppiech/Bug145635&id=06884c06bede0289785c08e75d2d4fcd41963421
Comment 43 Pawel Piech CLA 2013-01-23 00:48:32 EST
Created attachment 225964 [details]
Screenshot of pin drop-down.
Comment 44 Pawel Piech CLA 2013-01-23 00:52:05 EST
(In reply to comment #41)
> I don't have steps to reproduce.  Just was debugging a target Eclipse
> instance.
I've come across a couple of these before: the various viewer updates are racing with the control when its disposed.  I added a disposed guard which should take care of this instance.
Comment 45 Michael Rennie CLA 2013-01-23 11:00:54 EST
Couple of nits so far:

1. the var view does not initialize correctly if you pin it before starting debugging. 

Steps:
a. start eclipse, open var view, pin it
b. debug hello world
c. notice that the var view is empty with the breadcrumb still saying no active context - I would assume that when the frame is selected in the debug view it would force the var view to do the same if no active context is displaying there.

2. stepping seems to remove the frame context from the var view, leaving it empty after each step (i'll attach a screen shot)

3. there have been a few time when changing the frame in the var view just empties the view - i.e. does not show vars

4. in 4.3.0 - N20130122-2000 I am also getting a metric tonne of the widgetDisposed exceptions like Curtis mentioned
Comment 46 Michael Rennie CLA 2013-01-23 11:02:03 EST
Created attachment 225989 [details]
empty var view after step

This shows the var view after I stepped once from the breakpoint - notice it collapsed the thread
Comment 47 Michael Rennie CLA 2013-01-23 11:30:18 EST
I pushed the JDT debug patch to its own branch so we don't have to keep applying it:

http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/log/?h=mrennie/bug145635
Comment 48 Dani Megert CLA 2013-01-23 11:33:06 EST
(In reply to comment #37)
> Created attachment 225529 [details] [diff]
> Patch for JDT with memento providers for Debug view.
> 
> JDT needs a small addition to its memento providers in order to enable
> restoring of the pinned context in breadcrumb.

Is this really needed? If so, it means the feature won't work well for clients like JDT without adding code.
Comment 49 Michael Rennie CLA 2013-01-23 12:05:44 EST
getting an assertion failed exception:

org.eclipse.core.runtime.AssertionFailedException: null argument:Action must not be null
	at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:85)
	at org.eclipse.jface.action.ContributionManager.add(ContributionManager.java:75)
	at org.eclipse.debug.internal.ui.views.variables.VariablesView.fillContextMenu(VariablesView.java:1105)
	at org.eclipse.debug.ui.AbstractDebugView$2.menuAboutToShow(AbstractDebugView.java:537)
	at org.eclipse.jface.action.MenuManager.fireAboutToShow(MenuManager.java:343)
	at org.eclipse.jface.action.MenuManager.handleAboutToShow(MenuManager.java:475)
	at org.eclipse.jface.action.MenuManager.access$1(MenuManager.java:470)
	at org.eclipse.jface.action.MenuManager$2.menuShown(MenuManager.java:500)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:255)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4155)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1466)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1489)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1470)
	at org.eclipse.swt.widgets.Menu.menuWillOpen(Menu.java:806)
	at org.eclipse.swt.widgets.Display.windowProc(Display.java:5586)
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
	at org.eclipse.swt.internal.cocoa.NSMenu.popUpContextMenu(NSMenu.java:77)
	at org.eclipse.swt.widgets.Menu._setVisible(Menu.java:278)
	at org.eclipse.swt.widgets.Display.runPopups(Display.java:4078)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3633)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1053)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:942)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:86)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:588)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:543)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:353)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:180)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:601)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:629)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:584)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1443)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1419)

Steps:
1. start debugging on Mac OS
2. in the var view open the variable context menu using the control key (not the pretzel, the one that says control on it)+ left click
3. cancel the popup by clicking somewhere else in the var view

Will test without pin+clone to see if this is bug in general.
Comment 50 Pawel Piech CLA 2013-01-23 13:55:33 EST
As discussed this morning, it's safe to say that this enhancement should be post-poned until at least M6.

(In reply to comment #45)
> Couple of nits so far:
> 
> 1. the var view does not initialize correctly if you pin it before starting
> debugging. 
This is working as intended.  The view is supposed to stay pinned to whatever context you pinned it to.  If the debug view selection was empty when you pinned it, it will stay that way.

That said, I agree that this is not a good user experience.  I think the sensible thing to do would be to disable the pin action if there is no context to pin to.
 
> 2. stepping seems to remove the frame context from the var view, leaving it
> empty after each step (i'll attach a screen shot)
The breadcrumb is supposed to re-select the stack frame even if you resume and later suspend a thread.  Do you see this consistently stepping anything?

> 3. there have been a few time when changing the frame in the var view just
> empties the view - i.e. does not show vars
This also seems like a failure of re-selection logic.  
 
> 4. in 4.3.0 - N20130122-2000 I am also getting a metric tonne of the
> widgetDisposed exceptions like Curtis mentioned
Are they all in the same location?  Also, do you notice what action triggers them?


(In reply to comment #48)
> Is this really needed? If so, it means the feature won't work well for
> clients like JDT without adding code.
The pin breadcrumb uses a memento mechanism of the model elements to figure out which elements to re-select.  It is the same mechanism that the views use to restore the state (re-expand elements) of the variables and registers views when switching between contexts.  Since in this case it's the debug view state that needs to be restored, the various models which implement this part of the flexible hierarchy API, will need to be updated.  I have a patch for JDT and CDT, also if a debugger is strictly based on the standard debug model it will also be supported without additional changes.


(In reply to comment #49)
> getting an assertion failed exception:
This seems unrelated, though serious enough (I'm not able to reproduce here).  Are you able to debug into getAction() and see why null is returned for the find action?
Comment 51 Curtis Windatt CLA 2013-01-23 16:30:00 EST
Cloning a view opens it in the background of a tab group.  I don't have any immediate feedback that I did anything by pressing the button.  As you can hit it multiple times in a row it is easy to keep hitting the button.

In my example, I have variables as a minimized/popup view at the top right in vertical mode (covers entire right side of window when opened).  Each new variables view goes to the end of the stack at the bottom of my window.  The popup variables view hides the exact area of the screen the view is being added to.

I would imagine giving focus to the new view would be too disruptive.  Could we have the view open at the top of the stack?
Comment 52 Pawel Piech CLA 2013-01-23 17:25:03 EST
(In reply to comment #51)
> I would imagine giving focus to the new view would be too disruptive.  Could
> we have the view open at the top of the stack?

I guess just changing showView(IWorkbenchPage.VIEW_CREATE) to showView(IWorkbenchPage.VIEW_VISIBLE) does the trick.  I really wish we could have the new view be added to the same stack as the original, but I don't see a way to do that.
Comment 53 Dani Megert CLA 2013-01-27 08:00:39 EST
Markus and I looked at this together. We like the general direction and how it makes it easier to debug multi-threaded code. Some of the UI concepts need to be changed to better fit into the existing UI metaphor of Eclipse:

- Most people will use the original/existing view which reacts to the 
  selection. So, we best always keep that view as is and allow to open pinned 
  Debug breadcrumb controlled views from it (Open New Pinned View). Whether this
  appears in the view tool bar or the view menu depends on how often we expect
  users to use that functionality. I'm fine with a toolbar icon. The 'Pin' icon 
  should be removed.

- A newly opened pinned view needs to be activated and it should have the 'Pin'
  decorator in the view tab. No UI is needed to unpin or remove the debug
  breadcrumb.

- The Debug breadcrumb looks exactly the same as when used in the Debug 
  view. Therefore people will expect that it behaves the same i.e. that it also
  serves as the current scope for all the commands and actions. Either this 
  needs to be fixed or we need to use a slightly different UI. Markus and I were
  not yet sure which path to go. Markus tends to the former while I tend to the
  latter, but we are not 100% sure yet.

- This new feature should also be added to the Display view.
Comment 54 Pawel Piech CLA 2013-01-28 12:19:15 EST
(In reply to comment #53)
> Markus and I looked at this together. We like the general direction and how
> it makes it easier to debug multi-threaded code. Some of the UI concepts
> need to be changed to better fit into the existing UI metaphor of Eclipse:
> ...

Overall I like the changed paradigm, the main goal of the feature is to allow user to look at data from different contexts side by side and arguably this proposal is somewhat more efficient in achieving that.  
It's somewhat ironic though that you say you want it to fit this feature to the  Eclipse paradigm better because the "pin and clone" workflow mimics other pinnable views in Eclpse (like the console). 

Unless Patrick or anyone else CDT has a very strong argument against, I'll rework the patch for these suggestions, but I have a couple of clarifying questions:

> - A newly opened pinned view needs to be activated and it should have the
> 'Pin' decorator in the view tab. No UI is needed to unpin or remove the debug
>   breadcrumb.
If we are getting rid of the pin paradigm do we still want to use a pin decorator?  Also, do you have any ideas on what label/icon to use for the new pin action?

> 
> - The Debug breadcrumb looks exactly the same as when used in the Debug 
>   view. Therefore people will expect that it behaves the same i.e. that it
>   also serves as the current scope for all the commands and actions. Either 
>   this needs to be fixed or we need to use a slightly different UI. 

I think this implies having the breadcrumb have its own context menu.  I also like this idea as it makes the feature a lot more powerful, though I'm somewhat intimidated about the added complexity.  I'll look into it more.
Comment 55 Markus Keller CLA 2013-01-28 14:44:39 EST
The existing solutions for "pin" are already quite a zoo. Most of them are accompanied by an "open new view" action in some form, and I don't think the "pin & release" toggling is really convenient. Not to speak of the trouble with workbench window layout and how stuff works after a restart. A better paradigm is to open a new view instance and then configure that one. The Console and the Search view also allow that workflow, and for the Problems view, that's the only one.

> > - A newly opened pinned view needs to be activated and it should have the
> > 'Pin' decorator in the view tab. No UI is needed to unpin or remove the debug
> >   breadcrumb.
> If we are getting rid of the pin paradigm do we still want to use a pin
> decorator?  Also, do you have any ideas on what label/icon to use for the
> new pin action?

We should get rid of the pin/unpin paradigm that adds a mode to the UI (state of the pin button determines what happens when the next view input change occurs). But the additional view instances could still be called "pinned", since they don't take their input from the Debug view.

In the old days, there was a "pin" on the editor icon, but that's broken in 4.x (bug 390981). I guess it was /org.eclipse.ui/icons/full/ovr16/pinned_ovr.gif .

The action to open a new view doesn't need the "pin". It should just be the view icon with a '+' overlay.

> > - The Debug breadcrumb looks exactly the same as when used in the Debug 
> >   view. Therefore people will expect that it behaves the same i.e. that it
> >   also serves as the current scope for all the commands and actions. Either 
> >   this needs to be fixed or we need to use a slightly different UI. 
> 
> I think this implies having the breadcrumb have its own context menu.  I
> also like this idea as it makes the feature a lot more powerful, though I'm
> somewhat intimidated about the added complexity.  I'll look into it more.

The context menu wouldn't be my first priority. More important are shortcuts, window menu items, and window toolbar buttons.
Comment 56 Pawel Piech CLA 2013-02-01 00:44:43 EST
(In reply to comment #53)
> Markus and I looked at this together. We like the general direction and how
> it makes it easier to debug multi-threaded code. Some of the UI concepts
> need to be changed to better fit into the existing UI metaphor of Eclipse:

I updated the patch to reflect the main change requested in this comment:  the pin and clone actions are replaced by a single "Open View Pinned To Context" action.  The action still has the drop-down style which let's the user select which context provider to pin to, but given how the action is reworked it won't necessarily conflict with the legacy CDT pin/clone workflow so we could get rid of the drop-down without much conflict. The open icon is the same as the view icon with a "+" overlay.  The new view that is opened has a pin overlay.

On the implementation side, there's a lot of extra actions and cleanup, but I wanted to put the feature out there to try out before committing a lot of time to refactoring.
Comment 57 Pawel Piech CLA 2013-02-01 12:55:10 EST
(In reply to comment #56)
I found a problem with drawing the pin on top of a view icon: whenever I move the view to another stack, the pin disappears.  Tracking this down, I found that when creating the tab, the StackRenderer(SWTPartRenderer) creates the image from the URI rather than getting it out of the part.  I'm not sure if this is a bug in E4, but I actually can't think of any other view that changes its icon.
Comment 58 Markus Keller CLA 2013-02-01 13:29:10 EST
> but I actually can't think of any other view that changes its icon.

The JUnit view shows progress in the icon, but it's not the perfect test case, since it only shows progress while the view is not on top, and it updates the icon while tests are running. This is broken when the view is minimized (bug 384108).
Comment 59 Pawel Piech CLA 2013-02-14 19:41:35 EST
I pushed another update to the pin branch:
- After talking with Mike, I got rid of the dropdown for the open+pin action.  99% of the time this menu would only list the debug view and only serve to confuse the users.  I left all the APIs in place for an alternative pin provider and CDT can use those to add its own pin action.  Also, with the new workflow, CDT's existing pin and clone actions don't clash as much, which makes this a more backward compatible change.
- I moved Memory view's pin action back to the memory view toolbar, since it doesn't clash with the new open+pin action neither.
- I removed the pin decorator from the view icon.
- I cleaned up all the redundant pin actions.

The remaining TODO is the persistence of the pinned context between IDE runs.  I think I should work on that once we get this initial patch merged.

Dani, Mike, and Markus, and Patrick, could you verify that the new workflow is as you expected and that it's usable?  I'd like to merge this branch to master now so we can get some wider testing on it before we get too close to M6.
Comment 60 Pawel Piech CLA 2013-02-15 10:48:17 EST
(In reply to comment #55)
> The context menu wouldn't be my first priority. More important are
> shortcuts, window menu items, and window toolbar buttons.

I don't think we can make the data view's breadcrumb drive the global toolbar actions.  This would require that the view would be a debug context provider for the window, which would mean that when the data view got focus, all the other data views would switch to the active context in the pinned view.  That would kind of defeat the purpose of pinning the views besides making the debugger seem a little schizophrenic.

I think Dani's suggestion makes sense though: I could make the breadcrumb background gray, which would make it look less like the Debug view.  I'll try that out later today.
Comment 61 Patrick Chuong CLA 2013-02-15 14:12:23 EST
I pickup the latest from the side branch and spent sometime trying it out.
I have demonstrated the platform pin feature to our marketing team and here are the things that we would like to be able to do.

- Allow pinning from the original view without opening a new view. The new single button for opening a new view and pin it to the current debug context is a nice improvement, but would also like to be able to pin the original view too.

- The pin action uses the current selection in the debug view and sticks to that selection. For our debugger, we like to be able update the pinned view if any stackframe changes within the same thread. For example, when the user pins the Variables view with a stackframe selected, selecting any stackframe within the same thread will trigger the Variables view to update. And the view itself should display the thread name instead of the stackframe. To make it generic, debugger should be able to filter which selection event should be pass to the pinned view.

Do you have an updated patch for CDT so that I can experiment with the Disassembly view and the Memory Browser?

There seems to be a bug for the tree model view. If I pin/clone a view and the new view happens to open in an empty group, then the view is blank. You can reproduce this by closing all the views at the bottom stack and then clone a new Variables view.
Comment 62 Pawel Piech CLA 2013-02-15 16:21:39 EST
(In reply to comment #61)
> I pickup the latest from the side branch and spent sometime trying it out.
> I have demonstrated the platform pin feature to our marketing team and here
> are the things that we would like to be able to do.
> 
> - Allow pinning from the original view without opening a new view. The new
> single button for opening a new view and pin it to the current debug context
> is a nice improvement, but would also like to be able to pin the original
> view too.
The UI gurus decided (comment #53) that the combined action fits in better with the overall UI paradigm.  However, I want to leave the API used to pin a view public, so if in your product or in CDT, you decide that you still want an explicit pin action, it will be easy to add.

> - The pin action uses the current selection in the debug view and sticks to
> that selection. For our debugger, we like to be able update the pinned view
> if any stackframe changes within the same thread. For example, when the user
> pins the Variables view with a stackframe selected, selecting any stackframe
> within the same thread will trigger the Variables view to update. And the
> view itself should display the thread name instead of the stackframe. To
> make it generic, debugger should be able to filter which selection event
> should be pass to the pinned view.
Here also, I would like to have the APIs available, but let you add the extra UI elements in your product or CDT.  I started a "pin_clone" branch in CDT and I registered the CDT pin context provider there as well (the cdt side is not yet complete as this UI is still in flux).  
However, there's one major issue to consider: adding the use of these APIs to CDT will break CDT's compatibility with Platform 3.8/4.2, there was some discussion on cdt-dev about maintaining this compatibility, so we need to hash it out there before adding this dependency.

> There seems to be a bug for the tree model view. If I pin/clone a view and
> the new view happens to open in an empty group, then the view is blank. You
> can reproduce this by closing all the views at the bottom stack and then
> clone a new Variables view.
Thank you!  I reproduced it and will look into it.
Comment 63 Norman Yee CLA 2013-03-05 10:49:03 EST
Hi Pawel,

I tried out your pin and clone patch, and I've got a few comments:

1) Patrick on the CDT mailing list mentioned that it doesn't make sense to pin certain views like the Registers view to a stack context and those views should be pinned only to a core context.  We have the same issue with our debugger.  Is it possible to limit what the users can pin a view to?

2) If you pin the Variables view to the core(thread) node in the Debug view and then select a stack frame under the core node, the variables view doesn't update.

3) We're leaning toward using the legacy CDT pin and clone.  If we decide to use the legacy CDT pin and clone, is there a way to disable or remove the "Open New View Pinned to Context" toolbar button?
Comment 64 Pawel Piech CLA 2013-03-05 12:40:26 EST
(In reply to comment #63)
> Hi Pawel,
> 
> I tried out your pin and clone patch, and I've got a few comments:
> 
> 1) Patrick on the CDT mailing list mentioned that it doesn't make sense to
> pin certain views like the Registers view to a stack context and those views
> should be pinned only to a core context.  We have the same issue with our
> debugger.  Is it possible to limit what the users can pin a view to?
Short answer is no.  The pin control is written to be generic in respect to the view that it's placed in.  

> 2) If you pin the Variables view to the core(thread) node in the Debug view
> and then select a stack frame under the core node, the variables view
> doesn't update.
Do you mean, select the stack frame in Debug view or the breacrumb?  If it's the former, than it's intended behavior since once the view is pinned it's context is indepenent of the Debug view (i.e. you could close the Debug view), it it's the latter than it's a bug.

> 3) We're leaning toward using the legacy CDT pin and clone.  If we decide to
> use the legacy CDT pin and clone, is there a way to disable or remove the
> "Open New View Pinned to Context" toolbar button?
Not the way the action is contributed currently.  I can change it so that it's contributed declaratively (which I should do anyway), in which case you could turn off the action using capabilities.
Comment 65 Pawel Piech CLA 2013-03-07 12:20:50 EST
The deadline to commit this feature for M6 is tomorrow and I haven't got a comprehensive review from another committer yet.  Also, I'm not getting much positive feedback on the feature from the guys that are using pin and clone in CDT right now, which is really the target audience for it.  On top of that, I'm still seeing some stability issues in the logic that re-selects pinned contexts, meaning that it'll still take a bit of fine tuning to work out the bugs in this feature in Kepler.

So unless someone complains really loudly right now and (and offers to help with making it rock solid), I'm going to bump this feature from Kepler.
Comment 66 Norman Yee CLA 2013-03-07 13:22:42 EST
(In reply to comment #64)
> > 2) If you pin the Variables view to the core(thread) node in the Debug view
> > and then select a stack frame under the core node, the variables view
> > doesn't update.
> Do you mean, select the stack frame in Debug view or the breacrumb?  If it's
> the former, than it's intended behavior since once the view is pinned it's
> context is indepenent of the Debug view (i.e. you could close the Debug
> view), it it's the latter than it's a bug.

In my custom DSF debugger, I have a core node as the parent of the stack frame nodes in the Debug view.  When I pin to the code node, and then select one of the stack frame nodes under it, I don't see the variables view update.
 
> > 3) We're leaning toward using the legacy CDT pin and clone.  If we decide to
> > use the legacy CDT pin and clone, is there a way to disable or remove the
> > "Open New View Pinned to Context" toolbar button?
> Not the way the action is contributed currently.  I can change it so that
> it's contributed declaratively (which I should do anyway), in which case you
> could turn off the action using capabilities.

Yes, please contribute it declaratively in the plugin.xml file.  That would make it possible to hide it in our debugger.  Thanks.
Comment 67 Pawel Piech CLA 2013-03-07 16:55:26 EST
(In reply to comment #66)
> In my custom DSF debugger, I have a core node as the parent of the stack
> frame nodes in the Debug view.  When I pin to the code node, and then select
> one of the stack frame nodes under it, I don't see the variables view update.
I'm sorry, but your statement is still a little ambiguous: are you selecting the stack frame in the breadcrumb or in the Debug view?  If it's with the breadcrumb, then does it update when you select a different core, or when you switch stack frames?

> Yes, please contribute it declaratively in the plugin.xml file.  That would
> make it possible to hide it in our debugger.  Thanks.
The only down side to contributing it declaratively is that the icon couldn't be customized per-view as it is now.  In any case, I'd rather the feature meet the needs of the community with the feature from the outset and if we have vendors that need to hide it then it's a clear sign that it's not the case.  If there's continued interest in this feature, we should revise it accordingly.
Comment 68 Norman Yee CLA 2013-03-08 16:34:22 EST
(In reply to comment #67)
> (In reply to comment #66)
> > In my custom DSF debugger, I have a core node as the parent of the stack
> > frame nodes in the Debug view.  When I pin to the code node, and then select
> > one of the stack frame nodes under it, I don't see the variables view update.
> I'm sorry, but your statement is still a little ambiguous: are you selecting
> the stack frame in the breadcrumb or in the Debug view?  If it's with the
> breadcrumb, then does it update when you select a different core, or when
> you switch stack frames?

I'm selecting the stack frame in the Debug view, and the Variables view doesn't update.

If I select a stack frame in the Variables view's breadcrumb, I see the view update.

> > Yes, please contribute it declaratively in the plugin.xml file.  That would
> > make it possible to hide it in our debugger.  Thanks.
> The only down side to contributing it declaratively is that the icon
> couldn't be customized per-view as it is now.  In any case, I'd rather the
> feature meet the needs of the community with the feature from the outset and
> if we have vendors that need to hide it then it's a clear sign that it's not
> the case.  If there's continued interest in this feature, we should revise
> it accordingly.

Couldn't you contribute the toolbar button declaratively per view (e.g, registers view, expressions view, variables view, etc.) like the way the CDT pin and clone currently works?  That way, you can have a different icon for each view.
Comment 69 Pawel Piech CLA 2013-03-08 16:45:11 EST
(In reply to comment #68)
> I'm selecting the stack frame in the Debug view, and the Variables view
> doesn't update.
> 
> If I select a stack frame in the Variables view's breadcrumb, I see the view
> update.
Ah, then it's working as intended, though not as you were expecting.

> Couldn't you contribute the toolbar button declaratively per view (e.g,
> registers view, expressions view, variables view, etc.) like the way the CDT
> pin and clone currently works?  That way, you can have a different icon for
> each view.
Of course, I guess I was getting attached to the overlay icon I made :-)
Comment 70 Pawel Piech CLA 2013-03-11 13:11:36 EDT
*** Bug 372354 has been marked as a duplicate of this bug. ***
Comment 71 Martin Oberhuber CLA 2013-03-19 11:40:39 EDT
We are interested in this feature, and we're wondering what it would take to get this into Kepler.

1. Can we have a comprehensive list of the Platform Team's must-haves ?
2. Can we get a ROM estimate of the effort needed to meet those must-haves ?

I understand that M6 was the official API freeze, and M7 is feature freeze. I don't exactly understand in what respect this feature adds API. Could any API perhaps be marked as provisional in order to comply with API rules for M7 ?
Comment 72 Dani Megert CLA 2013-03-19 11:58:17 EDT
(In reply to comment #71)
> We are interested in this feature, and we're wondering what it would take to
> get this into Kepler.
> 
> 1. Can we have a comprehensive list of the Platform Team's must-haves ?

See comment 53 and comment 55. I don't know how much of this is already  adopted in Pawel's branch. I'm currently too busy to spend time on this.

Besides the API changes, the other concerns are:
- the changes are really big, not just in the LAF but also in the code and will
  cause ripples (see bugs reported in this bug here already)
- Pawel notes in comment 65: Also, I'm not getting much positive feedback on the feature from the guys that are using pin and clone in CDT right now, which is really the target audience for it. 


Doing this early Luna, so that all interested parties can get satisfied and test it, and we have time to polish the code and the LAF, is the right approach here.
Comment 73 Pawel Piech CLA 2013-03-19 13:19:33 EDT
(In reply to comment #72)
> (In reply to comment #71)
> > We are interested in this feature, and we're wondering what it would take to
> > get this into Kepler.
> > 
> > 1. Can we have a comprehensive list of the Platform Team's must-haves ?
> 
> See comment 53 and comment 55. I don't know how much of this is already 
> adopted in Pawel's branch. I'm currently too busy to spend time on this.
I believe addressed all the issues raised in these two comments, with the exception of pin decorator for the view (I discussed with Mike and decided it was a bad idea), and the Display view (didn't get that far).

But I had two major concerns left:
1) The CDT folks that are using pin and clone now indicated that they were going to try to hide this feature and basically work around it in CDT.  This seems like a red flag this late in the release.
2) When testing with CDT and TCF, and I still saw some issues with the logic that re-selects the elements after terminating and re-launching.  I didn't want to commit to tracking down those issues in M7 (in addition to updating CDT) because I can't really give it 100% of my time.
Comment 74 Lars Vogel CLA 2019-11-27 07:18:53 EST
This bug hasn't had any activity in quite some time. Maybe the problem got
resolved, was a duplicate of something else, or became less pressing for some
reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it.
The information can be, for example, that the problem still occurs, that you
still want the feature, that more information is needed, or that the bug is
(for whatever reason) no longer relevant.

If the bug is still relevant, please remove the stalebug whiteboard tag.