Community
Participate
Working Groups
Build Identifier: I20111209-1447 The existing Expressions view shows all expressions present in the workspace as a simple list. Sometimes the user has multiple projects in his workspace, and has created expressions for all of them. While debugging a single project, it is inconvenient to see all those expressions in the view which are not relevant for this project. The view tries to evaluate all expressions, and most of them fail with an error message. Working sets give us an elegant way to group relevant expressions together, and show only those expressions which are applicable for the project being debugged currently. The user can define working sets and assign expressions to each working set. He/she can assign the same expression to multiple working sets. Each expression view will use some or all of the working sets. This bug will track the implementation of this feature as proposed and as it evolves. Reproducible: Always
This feature will add working set support only in the DSF layer, not in the platform debug layer. So working sets will be visible in the Expressions view only when a dsf debug launch is present in the Debug view. Working sets will not be visible when the Debug view is empty, or a non-DSF debug launch is present.
Define working sets: A menuitem "Define working sets ..." is added to the dropdown menu of the Expression view. This menuitem opens a wizard. Here the user can create working sets and assign expressions to a working set. This wizard does not apply the working sets to the underlying view. Working set definitions are not view specific. So if multiple clones of Expression view are open, all clones will share the same working set definitions. You can assign the same expression to multiple working sets. This wizard is a bit different from the platform working sets UI. It offers all the definitions on a single page, the user does not have to open multiple dialogs to create working sets and assign expressions to a working set. Select working sets: By default, the Expression view shows all expressions present in the workspace. To apply working sets to a view, there will be a button on the view toolbar. This button opens a custom dropdown checklist of all working sets. The user can check multiple working sets to apply to the view. Each expression view will remember the applied working sets in its persistent view memento. When the view is in "working sets" mode, a default working set by the name "Others (no working set)" will be shown. This working set will contain all expressions which have been added thru the "Add new expression" cell when the expression view was in "all expressions" mode.
Created attachment 211378 [details] Screenshot of the "Define expression working sets" wizard
Created attachment 211379 [details] Screenshot of view toolbar button to select working sets, and the custom dropdown checklist of working sets
Thanks Abeer! I look forward to seeing the patch. You can post it here, or in a branch on github, whichever is easier for you. One thing you don't mention is what criteria you use to associate a working set with a context.
I think creating a branch in github will be easier because I can make changes as per suggestions in this bug. Once the final polishing is done, can create a patch from github and post the patch here.
Currently there is no association of a working set with a context. Because expressions in eclipse platform are just strings, without any meta information like the project, resource, etc. (like for breakpoints). So the user has to group expressions into working sets arbitrarily based on his understanding of the application being debugged and then apply given working set(s) to an expression view. The expression view just remembers the working sets applied to it by persisting working set names in the view memento.
Have created a fork of cdt on github and a new branch for this feature https://github.com/abeerbagul/cdt/tree/expression_workingset
Thank you Abeer for contributing this patch. It's a very interesting take on the feature and I think it has a lot of promise to make the expression view more useful. I played with the feature and browsed through the implementation and I have a lot of raw notes: 1) The working set wizard is quite different from the platform one. It's rather jarring at first, but once I got used to it, I agree that it's more convenient. Still I think consistency is important for a standard feature so it may be good to try to push some of your ideas to the standard working set dialogs instead. 2) The working set doesn't quite do what I expected it to. For me to have this feature be really compelling, it needs to automatically switch between sets depending on my debugging context. I.e. I'd like a way to pin a working set to a frame/thread/target. I don't think it would take much to extend the feature in this direction though. 3) The Select Working Sets action in the view toolbar is not a standard drop down menu nor a standard dialog. I actually had to look at the implementation to figure out what it is. It works as intended, but it's non standard. 4) The working sets are shown only when a DSF debug session is active. I understand that this is a known limitation, but I'm pretty sure our users would take it as a bug. 5) The working sets are shown as sub-trees in the view which confused me because the implementation uses a viewer filter. I also couldn't find a way to show just a single working set with out the sub-tree. I wonder if this is an artefact of the evolution of this feature. 6) If the working sets were not shown as sub-tree, but instead a viewer-filter was used, then it should be possible to have the filter apply even when a DSF debugger is not active. 7) There is a lot reliance on the knowledge of the platform internals. We can tolerate a little bit of such references, but only if there's a path to get rid of them. As an example, the branch does not build any more for me because I'm using the latest platform Juno I-build and one of the internal classes that the feature relies on has been removed. I also had some NPEs that I had to fix. They were probably a result of some other platform change (I'm using 4.2 BTW). 8) No javadoc comments. I think this is fine for a prototype though. Overall, I'm not sure what the next step should be for this feature. If I had time to put on this feature myself, I would try to develop it for work on platform and to use as much standard working set APIs as I could. Then I would focus on getting the feature to pinning feature from 2) above, because that is specifically a feature we get a lot of requests for from the community. But I'd really like to hear from others about their impressions of the feature as its implemented here.
Thanks a lot Pawel, for the extremely detailed analysis. Sorry for the delay in reply, was caught up in fixing bugs in our product :) Please see my replies to the notes in comment 9: Note 1: The working set wizard is quite different from the platform one. It's rather jarring at first, but once I got used to it, I agree that it's more convenient. Still I think consistency is important for a standard feature so it may be good to try to push some of your ideas to the standard working set dialogs instead. Reply: The wizard is a requirement for our product. I agree that for the CDT, it is better if expression working sets are presented through the standard working set UI to maintain consistency. I will add the standard UI for expression working sets to this patch. However, the standard UI for working sets is pretty clunky, even for projects, breakpoints, etc. Is it possible to file a bug for enhancing the standard working sets UI to make it more usable? So that can be a different bug and patch, for the eclipse platform working set UI. 2) The working set doesn't quite do what I expected it to. For me to have this feature be really compelling, it needs to automatically switch between sets depending on my debugging context. I.e. I'd like a way to pin a working set to a frame/thread/target. I don't think it would take much to extend the feature in this direction though. Reply: The current design is that user has to manually select the working sets in Expression view. I am willing to do the work to pin a working set to a frame/thread/target, but some guidance and help will be much appreciated since it is difficult for me to understand how to enable pinning for these working sets. How to implement pin for expression working sets need some input for the community, since an expression is a simple string, unlike breakpoints which have some context (such as IFile) associated with them. 3) The Select Working Sets action in the view toolbar is not a standard drop down menu nor a standard dialog. I actually had to look at the implementation to figure out what it is. It works as intended, but it's non standard. Reply: This is a requirement for our product. I can keep it as a separate optional addition if anyone else wants to integrate it into their product, or this can be a part of the enhancement to standard working sets UI as discussed in reply 1. 4) The working sets are shown only when a DSF debug session is active. I understand that this is a known limitation, but I'm pretty sure our users would take it as a bug. Reply: This bug can be fixed if working set support for expressions is added to eclipse debug platform similar to what is done for breakpoint working sets. I am willing to do the work, however, some guidance and help will be needed. 5) The working sets are shown as sub-trees in the view which confused me because the implementation uses a viewer filter. I also couldn't find a way to show just a single working set with out the sub-tree. I wonder if this is an artefact of the evolution of this feature. Reply: This is a requirement for our product. There can be a preference to toggle off this sub tree based layout, in favor of a simple list of filtered expressions. In that case, we need to filter out duplicate expressions, i.e. the same expression added to two working sets should not appear twice. In the simple list case, if the user wants to show two or more working sets, we also need to figure out how the "Add new expression" node will add a an expression and include it in multiple working sets. 6) If the working sets were not shown as sub-tree, but instead a viewer-filter was used, then it should be possible to have the filter apply even when a DSF debugger is not active. Reply: We had tried this approach in our product, and as described in reply 5, there can be a preference to toggle on this layout. This layout has the advantage that it can be applied even when a DSF debugger is not active. It has the disadvantage that the "Add new expression" node does not know where to add an expression if multiple working sets are being applied to the Expression view. 7) There is a lot reliance on the knowledge of the platform internals. We can tolerate a little bit of such references, but only if there's a path to get rid of them. As an example, the branch does not build any more for me because I'm using the latest platform Juno I-build and one of the internal classes that the feature relies on has been removed. I also had some NPEs that I had to fix. They were probably a result of some other platform change (I'm using 4.2 BTW). Reply: I need to fix this, these references are a result of my low knowledge in some areas. Some pointer will be helpful to clean up the code. 8) No javadoc comments. I think this is fine for a prototype though. Reply: I need to fix this.
Round 2 of this feature, integrating Pawel's analysis from comment 9, will include the following: 1. The standard eclipse platform UI will be used for creating working sets and assigning expressions to working sets. The standard UI will also be used to select a working set in the Expression View. 2. Only one working set can be applied to the view at a time. This is required to support the UI for "Add new expression" node, so that this node knows which working set to add the new expression into. 3. The sub-trees will not be shown, instead a flat list of only expressions from the one selected working set will be shown. 4. A viewer-filter will be used to filter in only those expressions included in the working set. This filter will apply even if the DSF Debug session is not active.
(In reply to comment #11) This looks very interesting ! I was surprised about some of the "Round 2" approaches since it looks like they are in contradiction with what you said were requirements for your product? Can you outline what workflows would suffer by aligning with Eclipse Standard dialogs versus doing your own thing as the current patch does ? > 1. The standard eclipse platform UI will be used for creating working sets and > assigning expressions to working sets. I'd suggest also considering the workflow where I select an expression in the view and "Right-click > Assign Working Sets..." like JDT does in the Package Explorer. I've always found that workflow a lot better than making assignments in a separate Workingset UI only, or at least it's a very valuable addition. > 2. Only one working set can be applied to the view at a time. That's a very bad Idea IMO sine I expect working sets to be created for various debug problems, but these are very likely overlapping so I'll want multiple ones active at any one time. Having multiple active is also Eclipse standard in the Project Explorer. The "Add new expression" problem would IMO better be solved by an artificial implicit "Other" working set which holds those expressions not assigned any working set (like JDT does in Package Explorer). Together with a "Show Workingsets as Toplevel Elements" that's a very powerful paradigm IMO. On a related note, have you considered "Import / Export Workingsets" for expressions? - That would be an extremely important piece IMO in order to share valuable expressions for particular debug problems in a team. > 3. The sub-trees will not be shown, instead a flat list of only expressions > from the one selected working set will be shown. Did you consider a "Show toplevel elements : workingsets" like in Project Explorer - it's IMO the best of both worlds (ie no full tree but also not entirely flat). I'm not exactly sure about the "avoid duplicate expressions" problem. Is it due to performance issues or anything else ? > 4. A viewer-filter will be used to filter in only those expressions included > in the working set. This filter will apply even if the DSF Debug session is Supporting debug sessions other than DSF is top important IMO. I understand that pushing parts of the contribution into Platform is required for that. Can you identify the partial patch that would be needed in Platform ? Finally, regarding Pawel's request to automatically associate workingset(s) with active debug context(s) by "pinning" ... did you look at what Mylyn is doing in that area with its frameworks? The problem seems similar to me: static Workingsets existed for a long time in Platform, but Mylyn added more dynamic filtering on various levels, associating those filters/groups with contexts ("tasks" in Mylyn) and activating / deactivating those contexts. I like the general Mylyn UI element (the 3 circles) which allows me to easily switch between "task-focused / filtered" view and "full view of everything" , as well as the way how Mylyn automatically determines what things (expressions in that case) may be "interesting" or "relevant" for me in the given context.
CQ:WIND00156833
The requirement of our product led to the patch attached in comment 8. However, the requirement for CDT is to follow the standard eclipse platform UI for working sets, so I will rework the patch to use standard UI. Our product will continue to use the custom UI, but we will maintain it separately in our product copy of CDT. If we can design some improvements to the eclipse platform UI for working sets, as a separate bug and patch, I am willing to do the implementation. At that point, we can re-evaluate whether to use the new UI in our product. However, at this point, it is better to contribute the patch as a starting point and keep a different UI in our product.
I will add the UI to select an expression in the view and "Right-click > Assign Working Sets...". If we allow multiple working sets in the Expression View, we need to solve the problem of deciding how "Add new expression" will behave. One way is that "Add new expression" will always add the new expression to "Others (no working set)". Expressions which are not assigned to any working set will be in "Others", and will always be visible in the view. The user can then move the newly added expression to any working set. Expressions from "Others (no working set)" should always be visible in the view, otherwise, new expressions typed in by the user will disappear. One behavior of the eclipse debug platform is that it allows duplicate expressions. I do not know whether this is a bug or a feature. An expression is just a string, without any metadata (unlike breakpoints), so how do duplicate expressions help the user? I can add a filter to "Show / hide duplicate expressions", or we can modify the behavior of "Add new expression" to disallow duplicate expressions. Duplicate expressions do not lead to performance issues, but I feel they confuse the user because there is no distinguishing factor. In a flat list, we need to add a column to the view "Working set" which will display the name of the working set this expression is contained in. This column will only appear when a DSF debug session is active. To make this column visible always, we need to add a small patch to the eclipse debug platform Expression view. Also, for the feature "Show toplevel elements : workingsets", we need to add a patch to the debug platform. I am willing to implement the patches, if we decide to do so. It would be great if part of the patches to support this feature can be pushed to debug platform, so that the UI experience will be seamless across debug platform and DSF debug. I can create the initial patches required for this. Import / Export of expression working sets is a good idea, need to file a separate bug for this? For pinning a working set to an active debug context, user can select an expression and "right click -> Pin to <active debug context>". This menuitem will have two submenus, "Pin select expression", "Pin <working set name>". In this case, view will remain unpinned, but it will display only the following expressions: 1. Expressions / working sets pinned to the <active debug context>, i.e. the selection in the debug view 2. Unpinned expressions, and "Others (no working set)" The view should have a toggle toobar button or menuitem which toggles the view between two states: 1. Show expressions / working sets pinned to <active debug contex> 2. Show all expressions / working sets. When the user pins an expression to the <active debug context>, the view will automatically toggle its state to show only pinned expressions. If the user has selected an expression from "Others (no working set)", the right click -> "Pin to active debug context" menuitem will not have any submenus, i.e. "Others (no working set)" itself cannot be pinned. I havent worked in Mylyn, can someone with experience in Mylyn describe how the pinning feature can be enhanced for Mylyn? Patch version 2, will be attached to this bug, rather than on github.
Created attachment 213745 [details] Spec describing expression working sets. Am creating a simple spec, to capture all the design and implementation points as being discussed in the bug. Can start implementation of draft 2 of the feature, once the spec reaches an initial level of stability as agreed upon by everyone. Please feel free to edit the spec directly, or suggest changes to it via comments. I will keep the spec updated as the feature evolves thru discussion and implementation.
(In reply to comment #15) Many thanks for creating the spec! I will run this by a couple reviewers. IMO the biggest design decision is whether multiple working sets can be active at any one time, or only one. I've been arguing for allowing multiple so far since that's how other parts of Eclipse work, but reading through things again there's benefit in the simplicity of allowing only one. I'm not yet fully decided on this one. Anyways here's some comments on the current state of things: > I can add a filter to "Show / hide duplicate expressions", or we can modify > the behavior of "Add new expression" to disallow duplicate expressions. I don't like the "Show/hide duplicate" button since I can't see any value in showing duplicates. IMO when I enter a duplicate, chances are that I just didn't find the original expression. So just bringing it to top of the view and/or adding to currently visible workingset(s) is most likely desired. See also my thoughts on pinned-to-context below. In general I'd recommend to not forcefully hide duplicates already in the view (may come from an old imported workspace; user can delete duplicates himself, or live with the duplicates) but add functionality instead which avoids adding more duplicates. > In a flat list, we need to add a column to the view "Working set" which will > display the name of the working set this expression is contained in. Why ? I don't think I need to know what workingsets an expression is in. Project Explorer and Package Explorer also don't show that information. IMO the extra column would cause more trouble than value. > When the user pins an expression to the <active debug context>, the view will > automatically toggle its state to show only pinned expressions. Having pinned an expression, a good workflow is needed to add more predefined (but currently invisible) expressions to the pinned context. An interesting approach would be using content assist for that: Click into an empty row, Ctrl+Space shows all predefined-but-invisible expressions; as I start typing, this is filtered down; pressing enter the predefined expression gets added to the pinned context. This would in fact work very much like a history of predefined expressions, and could also include expressions that were entered before but deleted later. The content assist popup could show multiple categories for that: Consider how the "Quick Access" box in the top right in Eclipse 4.2 works. In addition to that, an "Add Expressions From..." UI would make sense, allowing to bring in multiple previously-defined expressions from other workingsets or external file(s) in a single step. Such an "Add From..." UI could benefit from incremental filtering (eg show me all that use variable "Foo" then select all those). Also note that the filtered set of expressions-pinned-to-context creates another implicit workingset. So "convert pinned to workingset" would be an interesting gesture, just giving the currently-pinned a name. The more I think about it, the more I like this "pin to context" idea. I could see a number of expressions which are only locally valid to not be assigned any workingset but only be shown inside pinned context; and others, which access global/static variables to be defined in named workingsets which are added to the pinned context. > From the Spec: The Expression View will have two menuitems: > Select Working Sets > Deselect Working Sets I don't think a separate "Deselect" is needed. Eclipse project explorer or breakpoints view has a single "Select Workingsets..." for both.
(In reply to comment #17) I so far agree with where the discussion is going, there's just a a lot of details to work out. I copied the contributed spec to wiki: http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/ExpressionWorkingSets and suggest that we maintain it there rather than in this thread. I'm still trying to digest all the points, but one this I'll add now is that we can handle the problem which working set to add a new expression to by extending the "Add New Expression" editor and dialog. However, we would still have an ambiguity when adding via drag and drop or from context menus.
(In reply to comment #18) > I copied the contributed spec to wiki: > http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/ExpressionWorkingSets > and suggest that we maintain it there rather than in this thread. +1 Thanks!
Adding a new expression: If the Expression view has no working sets selected, the new expression will be added to eclipse expression manager and will simply become visible in the view. This is the default behavior of platform. If the view has a single working set applied, the new expression will be added to eclipse expression manager, as well as it will be added to the selected working set. If the Expression view has multiple working sets selected, a new expression can be handled in the following manner: 1. Add the new expression to all the selected working sets. 2. Open a popup dialog with a textbox where the user can edit the expression, and a checkbox list of all selected working sets, with the first working set pre-checked. The user can then click OK and the new expression will be added to eclipse expression manager as well as the checked working sets. 3. Create a dialog similar to the "Select default working set" dialog for breakpoints. If the view has multiple working sets, and no default working set, the first time a new expression is added, this dialog will open, prompting the user to select a default working set. The new expression will be added to this working set. Further new expressions will be added to this working set. 4. Maintain a built in working set "Others (no working set)", which is not deletable. All expressions not assigned to any working set, will belong to this Others working set, and will be always visible in the Expression view. A new expression will always be added to Others working set, and from here can be assigned to any working set.
> > From the Spec: The Expression View will have two menuitems: > > Select Working Sets > > Deselect Working Sets > > I don't think a separate "Deselect" is needed. Eclipse project explorer or > breakpoints view has a single "Select Workingsets..." for both. The Project Explorer does have a menu item "Deselect Working Set" in the dropdown menu of the view. However, if we decide not to provide this menu item in Expression view, I'm fine with that.
Am updating the wiki now as modifications to the spec come in.
I added a new section with an alternative proposal for pinning: http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/ExpressionWorkingSets#Pinning:_Link_Mode Also as first steps, it would suggest adding working sets concept to the org.eclipse.debug.internal.core.ExpressionManager, then add dialogs for selecting and editing working sets.
(In reply to comment #20) On adding new expressions: I think the most important thing to maintain is that adding a new expression must be quick and easy. As I'm deep into debugging an issue, I want to quickly add an expression without having to think about what working set to put it into ... having to think about that would completely break my flow. Therefore: > If the Expression view has no working sets selected, the new expression > will be added to eclipse expression manager and will simply become visible This is good. > If the view has a single working set applied, the new expression will be > added to eclipse expression manager, as well as it will be added to the > selected working set. Not good IMO. To me, workingsets are a more persistent mechanism organizing my stuff. As I just quickly add a new expression I don't want to care (yet) about how to organize it. It is _possible_ I want to have it added to the workingset, but I might also just want to have it in some temporary place for now. IMO the view needs to be deterministic and consistent; so regardless of whether one or two workingsets are displayed, the same thing needs to happen. Project Explorer and others also don't add to a workingset automatically. I think it is better not adding a new expression anywhere but maintain it in "Others" instead... adding to a workingset is a simple single gesture AFTER my debug session is done, using a multiselect and "Assign to Workingset" gesture. > If the Expression view has multiple working sets selected: > 1. Add the new expression to all the selected working sets. Not good at all. This would completely break the workingsets idea. > 2. Open a popup dialog with a textbox where the user can edit Not good since breaking the debug flow. > 3. Create a dialog similar to the "Select default working set" dialog > working set. Further new expressions will be added to this working set. Not good - creates cognitive friction and breaks my flow. While debugging I want to focus on my expressions and not on where and how to organize them. > 4. Maintain a built in working set "Others (no working set)", which is not > deletable. All expressions not assigned to any working set, will belong to > this Others working set, and will be always visible This is the best approach.
The user's flow while adding new expression behavior has been nicely explained in comment 24. So the spec for Add new Expression will be: All the current UI flows for adding a new expression to the Expression view will remain as they are. A new expression will always be assigned to the default working set "Others (no working set)". Since all expressions in the working set "Others" are always visible in the view, the new expression will also appear in the view. When the user wants (later), he can select the new expression and right-click -> Assign to working set. If he assigns to a working set which is selected for the view, the new expression will remain visible. If he assigns to a working set which is not selected for this view, the new expression will disappear.
Pinning (Link mode): There already exists a toolbar button "Pin to debug context" in the view. Can we enhance this button to add a dropdown menu to it? The dropdown menu will have two menuitems: 1. Pin view to debug context 2. Pin working sets to debug context Both menuitems will be "toggle menu items" The second menuitem will be enabled only if the user has selected working sets for this view. Clicking this menuitem will associate the selected working sets with the active debug context, and also persist this association. Now if the user selects a different debug context in the debug view, the view will realise that its pin state "pin working sets to debug context" has been toggled on. It will search its persistent state for a list of working sets associated with this debug context. If such a list is found, it will restore those working sets. If such a list is not found, the view has two options: 1. Continue showing the previously selected working sets. This has the side effect that since the pin state "Pin working sets to debug context" is toggled on, the previously selected working sets will now be associated with this debug context also. 2. Realise that no working sets are selected for this debug context, so deselect all working sets for the view, and show all expressions.
Could the enhanced expression feature help with what we are trying to achieve here? http://wiki.eclipse.org/CDT/User/NewIn82 I was thinking that the user could create different expression groups for the different cores, and expand the one that is currently relevant. So, similarly to creating a working set with variables 'row' and 'slow', the user would create a group: "row, slow" as an expression. Since it will be shown as a collapsible group, the user can expand it when looking at the right core, and collapse it otherwise.
Created attachment 221912 [details] Patch to platform (org.eclipse.debug.ui) for working set support for expressions This is a patch which introduces working set support for expressions. This will enable you to group expressions into working sets, in platform debug. The main use of this feature is for cdt-dsf, so there is a patch on top of this which enables this patch for cdt-dsf. The cdt-dsf patch has been submitted thru gerrit, and link to that gerrit patch is given below: https://git.eclipse.org/r/8048
Thank you Abeer. I gave the patch a try and it works as I would expect it to. I think that even in its current form it adds enough value to work it in. My immediate follow on request for this feature is to be able to link the working sets to debug contexts so that they get automatically activated when needed. I'll review the patch in more details, though on quick read I see that there's some mixing of APIs and non APIs that will need to get sorted out.
Please let me know where the mixups have occurred, will separate out apis and resubmit the patch.
Hi Abeer, I read through the patch and I don't see any major issues. Mostly there's just a lot of missing comments, plus the few notes below. 1) The expression and expression.workingset packages are set as public. This is the reason for the API warnings. They need to stay private and if we need access to these in CDT, we'll need to invent interfaces to the relevant data. 2) IAddNewExpression interface seems to only be references as a marker, is there a need to have the set/get methods in it? 3) ExpressionWorkingSetFilterManager seems to use view title as key to storing working set mementos. Using view's seconary ID is more appropriate. 4) ExpressionWorkingSetFilter should extend TreeModelViewerFilter so that we don't get the painting artifacts related to lazy loading filters. Although it means we may need another marker interface to denote the expressions parent element. 5) On usability, it would be nice to have a "Assign Working Sets..." context menu action and a dynamic set of menu actions to activate working sets, as in the JDT's package manager. Is there anything else you were still planning to add to this feature?
In JDT Package Explorer view, the popup menu for a project has a menuitem "Assign Working Sets..." which opens a dialog where user can select working sets. I think opening a dialog would be a bit clunky, an alternative option can be to create a submenu for "Assign Working Sets". This submenu would list all expression working sets defined, and a menuitem "New working set". The menuitem "New working set" will open the dialog page for "New expression working set", with all expressions selected in the view, pre-checked in the "New expression" dialog.
Rest of the issues apart from 5 in Comment 31 have been addressed. Will upload a new patch once we decide on which approach to take for issue 5, either a dialog box like JDT, or a submenu as described in Comment 32.
(In reply to comment #32) > I think opening a dialog would be a bit clunky, I was actually thinking of the JDT feature that does use the menu, where recently selected working sets appear in the menu itself as checkboxes. In other words, I agree with your comments.
Created attachment 222396 [details] Patch with comments and Usability suggested in comment 31
Hi Abeer, FYI I'll try to get to this review next week.
Hi Abeer, I've been putting off looking at this patch because I'd like to investigate whether we can automatically select working sets based on the active debug context. The basic mechanism would be to take the current input into the expressions view and retrieve the memento from the view input using IElementMementoProvider. Then when a new input is set to the view, to compare the input's memento with the saved ones. I think the tricky part will be to figure out how to show the association in the working sets dialog. We don't have a good API to access the input element's label and icon. We could use element's IElementLabelProvider, but we'd need to call it in the context of the view that provides the debug context (i.e. the debug view). However, once the debug session is terminated and the debug view is gone, all we will have is the element's memento and no way to provide a label for it. As a compromise, we could just post-pone trying to describe the context that a working set is pinned to, and just indicate that the working set is associated with some context.
I put the working sets feature into a branch and did some refactoring to consolidate the logic a bit: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/log/?h=ppiech/Bug372181 I'll try to add the memento tracking mechanism next.
http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?h=ppiech/Bug372181&id=597289dc45ec68d383f69461a6af586bd3d59caf I started implementing the mechanism to automatically select working sets based on selected element memento. I'm not sure how useful it would be in its current form, since it selects the working set based on the selected stack frame memento, but the experiment shows that it is possible. Next steps: 1) Add a menu action to toggle the auto-select behavior on/off. 2) Test and track down race conditions. 3) Investigate saving restoring expansion state when switching working sets. 4) Add memento saving and comparison to all elements in selected path (stack frame, thread, debug target, etc.).
I tried out the code in branch ppiech/Bug372181 commit 597289dc45ec68d383f69461a6af586bd3d59caf dated 18 Jan 2013. A couple of questions regarding the behavior: 1. Expression view has a map of stackframe memento -> workingsets. This map is populated when user opens the Working Set selection dialog and checks a working set to use. It is not populated when the view is activated. When view is first activated, and there is no debug session, this view shows working sets persisted in the view memento. But when a debug session starts, since the map of "stackframe -> workingsets" is empty, the view now removes all applied working sets. We need a way to persist this map to view memento or some other storage. 2. If the user applies a workingset to the view when Debug view is empty, this selection should be the "default selected workingsets". Only if a stackframe memento has a particular another workingset mapped to it, the default selection should be overridden. Currently, if a stackframe does not have a mapped workingset, all workingsets are removed from the view. 3. The stackframe memento is of type "Frame.<level>.<sessionID>". This memento might not be useful since the user associates workingsets with a certain function and not a frame level. E.g. main() calls foo(), so foo() is at frame level 0 and main is at level 1. Now user applies a workingset "wsmain" for main and a different workingset "wsfoo" for foo. The map now contains "Frame.0.0 > wsfoo" and "Frame.1.0 -> wsmain". If user steps out of foo(), using F7, when debug suspends in main, Expression view displays wsfoo since main is now at level 0 and wsfoo is mapped to level 0. We need to create stackframe memento based on function prototype and not frame level, e.g. memento for main(): "Frame.main(int, char*).0" memento for foo(): "Frame.foo().0" However, I can now visualize what you were intending, and the idea certainly makes sense. Please correct me if any of the comments above are not relevant.
I updated the working sets feature so that it gives user the option whether to have the workign sets automatically enabled, and to serialize the saved working sets on exit. I think that at this point the feature can be merged to master for M6. (In reply to comment #40) > I tried out the code in branch ppiech/Bug372181 commit > 597289dc45ec68d383f69461a6af586bd3d59caf dated 18 Jan 2013. > > A couple of questions regarding the behavior: > 1. Expression view has a map of stackframe memento -> workingsets. This map > is populated when user opens the Working Set selection dialog and checks a > working set to use. It is not populated when the view is activated. > When view is first activated, and there is no debug session, this view shows > working sets persisted in the view memento. > But when a debug session starts, since the map of "stackframe -> > workingsets" is empty, the view now removes all applied working sets. > We need a way to persist this map to view memento or some other storage. I implemented serialization of the saved working sets in this version. > 2. If the user applies a workingset to the view when Debug view is empty, > this selection should be the "default selected workingsets". Only if a > stackframe memento has a particular another workingset mapped to it, the > default selection should be overridden. Currently, if a stackframe does not > have a mapped workingset, all workingsets are removed from the view. I think having a default working set this way could get too confusing. I imagine the user trying figure out how to change the default, where the correct way to do it (de-select everything in Debug view) is not intuitive at all. > 3. The stackframe memento is of type "Frame.<level>.<sessionID>". This > memento might not be useful since the user associates workingsets with a > certain function and not a frame level. > E.g. > main() calls foo(), so foo() is at frame level 0 and main is at level 1. Now > user applies a workingset "wsmain" for main and a different workingset > "wsfoo" for foo. > The map now contains "Frame.0.0 > wsfoo" and "Frame.1.0 -> wsmain". If user > steps out of foo(), using F7, when debug suspends in main, Expression view > displays wsfoo since main is now at level 0 and wsfoo is mapped to level 0. > We need to create stackframe memento based on function prototype and not > frame level, > e.g. memento for main(): "Frame.main(int, char*).0" > memento for foo(): "Frame.foo().0" This is a CDT issue. CDT should recognize that the memento for a stack frame is being requested by the expressions view and use the frame's function instead of frame level.
I developed the fix for CDT to allow automatic selection of working sets based on stack frame (will post to gerrit). I also fixed the mapping of stack frame mementos in the view then tested with CDT and JDT. All looks good, so I merged into master for M6 testing next week. There's no API introduced by this feature, so we could back it out should something go horribly wrong with it.
Corresponding CDT change: https://git.eclipse.org/r/#/c/11011/
The fix looks great. I was trying to do something like this, i.e. Frame.<functionname>, but this patch is way more comprehensive since it falls back to frame address in case debug info is not available.
The new support causes an NPE in the testsuite and fresh workspaces. See bug 402946 for the compete trace.
-1, testing on Version: 4.3.0 Build id: I20130310-2000 I find this feature very confusing: 1. it breaks the platform notion of what a working set is - a visible logical grouping of the elements you assign to it. In this case there is absolutely no UI affordance letting me know I have grouped anything or that any kind of filtering is taking place, or even what the filtering means. 2. It would be nice if there was a more explicit way to turn it on / off. Currently I am assuming that if you uncheck all of the working sets in the dialog then working sets are not being used? I would be good if there was a Group By menu entry so I could simply and explicitly turn on / off working set support. 3. It also was not immediately apparent how to remove an expression from a working set. It would be better if we followed the Package Explorer here, and allow you to assign or remove multiple working sets at once using an Assign Working Sets action (or similar). 4. It feels to me like the entire feature could have just been done as a viewer filter (which is how it behaves).
(In reply to comment #46) Another oddity was when I debugged a hello world Java app in an existing workspace - I selected an existing expression, assigned it to a new working set and it vanished from the view. The only way I could bring it back was to remove the working set
(In reply to comment #46) > -1, testing on I backed the the commit from master, let's see if we can hash out the issues for M7. > I find this feature very confusing: > > 1. it breaks the platform notion of what a working set is - a visible > logical grouping of the elements you assign to it. In this case there is > absolutely no UI affordance letting me know I have grouped anything or that > any kind of filtering is taking place, or even what the filtering means. I agree that grouping by working set would be a nice visual clue and I agree that the lack of visual feedback is an issue, but I don't think it has to be a requirement. As a precedent, the package explorer doesn't group by working set by default. It does list some of the working sets in the view menu with an accelerator, where the selected set has a radio button check. But if you select multiple sets the radio button indication disappears. Grouping is problematic for this feature, because it would need to be supported by all the debug models, meanwhile the feature is currently implemented using filters which means that it doesn't even introduce a new API. > 2. It would be nice if there was a more explicit way to turn it on / off. > Currently I am assuming that if you uncheck all of the working sets in the > dialog then working sets are not being used? I would be good if there was a > Group By menu entry so I could simply and explicitly turn on / off working > set support. Following the package explorer, would it be enough to add a "Deselect working set" action, plus have the working sets listed in the menu with an accelerator? > > 3. It also was not immediately apparent how to remove an expression from a > working set. It would be better if we followed the Package Explorer here, > and allow you to assign or remove multiple working sets at once using an > Assign Working Sets action (or similar). Makes sense, the submenu is somewhat confusing. > 4. It feels to me like the entire feature could have just been done as a > viewer filter (which is how it behaves). It is implemented using filters, but I think working sets are more appropriate for the story: "As a user with many expressions where only some of them are used in different functions, I want to be able to select a subset of expressions when in a given function."
(In reply to comment #48) > Grouping is problematic for this feature, because it would need to be > supported by all the debug models, meanwhile the feature is currently > implemented using filters which means that it doesn't even introduce a new > API. > As a simple affordance we could follow how the type hierarchy view works, it has a comment at the end of the type name at the top of the view '- in working set: <working set name>' > Following the package explorer, would it be enough to add a "Deselect > working set" action, plus have the working sets listed in the menu with an > accelerator? > That makes sense, and would be something most people would already be familiar with > It is implemented using filters, but I think working sets are more > appropriate for the story: "As a user with many expressions where only some > of them are used in different functions, I want to be able to select a > subset of expressions when in a given function." Still just feels like filters to me, not necessarily a 'working set'. I guess the part that confuses me here is how the working sets are actually assigned to 'a given function' and how do I change that behaviour? In the code it looks like whatever the debug context is you created the working set for is what it applies to until you delete it - seems like it might be easier to just have an expression apply to a context rather than have an expression apply to a working set that applies to a context. Then you could have a simple UI to say 'show all expressions' or 'filter based on context'. Either way I think we still need some sort of UI to be able to see what applies to what and what state the view is in. My two cents.
(In reply to comment #49) > As a simple affordance we could follow how the type hierarchy view works, it > has a comment at the end of the type name at the top of the view > '- in working set: <working set name>' This sounds very reasonable, we'd be sacrificing some UI space for the content description, but it would provide a clue from the changing expressions. > Still just feels like filters to me, not necessarily a 'working set'. I > guess the part that confuses me here is how the working sets are actually > assigned to 'a given function' and how do I change that behaviour? In the > code it looks like whatever the debug context is you created the working set > for is what it applies to until you delete it - seems like it might be > easier to just have an expression apply to a context rather than have an > expression apply to a working set that applies to a context. Then you could > have a simple UI to say 'show all expressions' or 'filter based on context'. We started out on this feature without the automatic linking of context to working set. I.e. user manually selects the working set, and this is what I believe Abeer is using in his product. I added the automatic selection of working set based on the context, because that's what users normally reference to select their working set. Still for some use cases, it still make sense to let the user manually choose a working set, e.g. when the expressions are relevant to a core rather than a function, and user is debugging multiple cores. > Either way I think we still need some sort of UI to be able to see what > applies to what and what state the view is in. My two cents. What if we go with the following two options: when user is manually selecting working sets: just use the content description: 'in working set: abc'. When automatic seleciton is enabled the description would be: 'in working set: abc; selected for main()'
(In reply to comment #50) > What if we go with the following two options: when user is manually > selecting working sets: just use the content description: 'in working set: > abc'. When automatic seleciton is enabled the description would be: 'in > working set: abc; selected for main()' That makes sense to me.
We had a long discussions on the CDT call about this feature. One thing that came up is that there is grouping feature that was added in Kepler in CDT which may overlap somewhat with this working sets proposal. Abeer is to review the grouping feature to see if it would address his requirements (and possibly make the working sets redundant).
Created attachment 228310 [details] CDT Expression Groups (In reply to comment #27) > Could the enhanced expression feature help with what we are trying to > achieve here? http://wiki.eclipse.org/CDT/User/NewIn82 > > I was thinking that the user could create different expression groups for > the different cores, and expand the one that is currently relevant. So, > similarly to creating a working set with variables 'row' and 'slow', the > user would create a group: "row; slow" as an expression. Since it will be > shown as a collapsible group, the user can expand it when looking at the > right core, and collapse it otherwise. I tried this again with CDT master and the collapsed state is remembered. This is really cool because it allows to create expression groups for different stack frames and manually expand the correct ones and collapse the wrong ones for each frames; after that, selecting the stack frame will restore the groups expansion appropriately, showing the user only the relevant expressions. Furthermore, collapsed expressions will not be evaluated, so no extra work is done for invalid expressions. The attached screenshot of CDT shows two groups in the expressions view with each group applicable to a different stack frame. When choosing either stack frame, the correct group expands while the other one collapses automatically.
For reference, the CDT expression grouping feature is described in: http://wiki.eclipse.org/CDT/EnhancedExpressions. And implemented under bug 381754 and bug 394408.
(In reply to comment #54) > For reference, the CDT expression grouping feature is described in: > http://wiki.eclipse.org/CDT/EnhancedExpressions. And implemented under bug > 381754 and bug 394408. The final result of the feature is described in our N&N: http://wiki.eclipse.org/CDT/User/NewIn82#Enhanced_Expressions
The expression group feature is very useful, and it is pretty quick and flexible as a grouping feature. If we can implement auto expand and collapse for expression groups, and associate groups with stackframe memento and persistence in the same way as done here for working sets, that would be a good alternative to working sets.
