Bugzilla – Full Text Bug Listing
|
Description
Markus Keller
One for me! The conditional breakpoints editor already shows a similar history with recent expressions. In the Variables view, the combo label would also be a good place to advertise the existence of the integrated "Display view" functionality in that pane. I guess the hardest part of this bug will be to come up with a good label. This change request emerged based on a scientific study on how developers debug, in discussion with Daniel Megert, Markus Keller, Sarika Sinha, and Moritz Beller. Link to the study - http://repository.tudelft.nl/islandora/object/uuid%3Abf3325ce-f246-4977-91bc-785f877347b8?collection=education Moving out. Analysis done, the support has to go in Platform Debug as the Details pane is defined in the org.eclipse.debug.internal.ui.views.variables.VariablesView. JDT/CDT or any implementer can define a property to say it needs the history combo. Behavior of combo will be similar to condition history combo. Details about Combo - Description - Choose a previously entered expression to assign and perform other actions Combo Entries - 1) the value of variable 2) Other expressions used for this variable 3) Global History with expressions used for other variables Suggestion for better Description is welcome. As CDT has very limited actions, can not go int detail. Will see if description can be specific to JDT which will give more words. New Gerrit change created: https://git.eclipse.org/r/135468 New Gerrit change created: https://git.eclipse.org/r/135536 Gerrit change https://git.eclipse.org/r/135536 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=1e13edbf3ba52a50bae74a261945d8b4f8217d60 Gerrit change https://git.eclipse.org/r/135468 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=ff4e9cdaa92f6ccd9b1b4ff03890a7c32afca773 (In reply to Sarika Sinha from comment #5) > Behavior of combo will be similar to condition history combo. > > Details about Combo - > Description - Choose a previously entered expression to assign and perform > other actions > Combo Entries - > 1) the value of variable > 2) Other expressions used for this variable > 3) Global History with expressions used for other variables I've just was surprised by this feature and was curious to test it out. However it went bad. I must confess, this way how it is implemented right now it is more distracting than helping, and I think this must be fixed or reverted. My biggest issue is that it seems to automatically remember all *selected* values and adds them to the "proposal" list. So while a short debugging session just selecting variables fills the list with nonsense "proposals". After that, one looses an overview *what* is the *actual* value if one selects something in the list. Second: if the variables are on the right stack, the combo and the proposal list is to narrow to be useful, can't be resized and so can't be fully read / used (if it would contain real expressions). Third: it allows to select those nonsense values to absolutely unrelated variables of different types and does not show that the value is actually not applied yet. There is no indication that the text in the detail pane is need to be "saved" first. I know this was like this before, but now it is really confusing. Also this combo appears now in the debug hover over variables? And allows me to select some value? But this makes no sense at all, one can't "apply" this value in the debug hover, there is no "Ctrl+S" or other way to do this, it is read-only.. Created attachment 277300 [details]
Read only hover with combo and "wrong" value selected from it
Created attachment 277301 [details]
Selected variable shows nonsense value proposed by combo
Created attachment 277302 [details]
To narrow list with nonsense values from previously *selected* variables
Created attachment 277306 [details]
Save changes in Variables?
It also looks like the change caused another strange regression: after debug session, on committing a change in Git Staging view, a "Save changes in Variables?" dialog appears, and disappears only if Variables view is closed. There is NOTHING in Variables view needed to be saved at this moment.
I think we should revert the change.
Thanks Andrey for trying out!! I am looking into the issues, based on the results we can decide how to proceed. Sarika, from my feeling, a combo between panes doesn'T fit & look good. WDYT about a content assist, like in the EGit Staging view, with the light bulb etc? This would fit for any view size and will not confuse. We had content assist in Detail pane of variables view but not many people were aware about it, so we also need to handle the discoverability part also. Where do you suggest the light bulb ? Created attachment 277326 [details]
light bulb in staging view
(In reply to Andrey Loskutov from comment #19) > Created attachment 277326 [details] > light bulb in staging view This can give the tip that the Detail pane has content assist, but how do we help the user in remembering the older values ? By the way we have local history and global history like conditions in the combo. (In reply to Sarika Sinha from comment #20) > (In reply to Andrey Loskutov from comment #19) > > Created attachment 277326 [details] > > light bulb in staging view > > This can give the tip that the Detail pane has content assist, but how do we > help the user in remembering the older values ? Automatically once Ctrl+S is executed we apply the condition and save it if it was applied without errors. (In reply to Andrey Loskutov from comment #11) > Also this combo appears now in the debug hover over variables? And allows me > to select some value? But this makes no sense at all, one can't "apply" this > value in the debug hover, there is no "Ctrl+S" or other way to do this, it > is read-only.. -1 for having a combo in the hover. (In reply to Andrey Loskutov from comment #19) > Created attachment 277326 [details] > light bulb in staging view We also show this e.g. in the Find/Replace dialog when Regex is enabled. I am working on disabling the combo from hover. Yes, the light bulb will help. New Gerrit change created: https://git.eclipse.org/r/135927 (In reply to Eclipse Genie from comment #25) > New Gerrit change created: https://git.eclipse.org/r/135927 Removes the combo from Hover. I am seeing following error in the gerrit build tests - 16:59:54 at org.eclipse.swt.SWT.error(SWT.java:4621) 16:59:54 at org.eclipse.swt.SWT.error(SWT.java:4510) 16:59:54 at org.eclipse.swt.SWT.error(SWT.java:4481) 16:59:54 at org.eclipse.swt.graphics.Cursor.<init>(Cursor.java:185) 16:59:54 at org.eclipse.swt.widgets.Display.getSystemCursor(Display.java:3009) 16:59:54 at org.eclipse.swt.custom.StyledText.<init>(StyledText.java:1247) 16:59:54 at org.eclipse.jface.text.TextViewer.createTextWidget(TextViewer.java:1676) 16:59:54 at org.eclipse.jface.text.TextViewer.createControl(TextViewer.java:1699) 16:59:54 at org.eclipse.jface.text.source.SourceViewer.createControl(SourceViewer.java:439) 16:59:54 at org.eclipse.jface.text.source.SourceViewer.<init>(SourceViewer.java:426) 16:59:54 at org.eclipse.jface.text.source.SourceViewer.<init>(SourceViewer.java:402) 16:59:54 at org.eclipse.ui.console.TextConsoleViewer.<init>(TextConsoleViewer.java:237) 16:59:54 at org.eclipse.ui.console.TextConsoleViewer.<init>(TextConsoleViewer.java:224) 16:59:54 at org.eclipse.ui.internal.console.IOConsoleViewer.<init>(IOConsoleViewer.java:59) 16:59:54 at org.eclipse.ui.internal.console.IOConsolePage.createViewer(IOConsolePage.java:73) 16:59:54 at org.eclipse.ui.console.TextConsolePage.createControl(TextConsolePage.java:153) 16:59:54 at org.eclipse.ui.internal.console.IOConsolePage.createControl(IOConsolePage.java:64) 16:59:54 at org.eclipse.ui.internal.console.ConsoleView.doCreatePage(ConsoleView.java:326) 16:59:54 at org.eclipse.ui.part.PageBookView.createPage(PageBookView.java:378) 16:59:54 at org.eclipse.ui.part.PageBookView.partActivated(PageBookView.java:718) 16:59:54 at org.eclipse.ui.internal.console.ConsoleView.lambda$0(ConsoleView.java:412) 16:59:54 at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40) 16:59:54 at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185) And the build time outs - 16:59:54 ... 46 more 17:17:50 Waiting for evaluation to start.. 18:06:04 Build timed out (after 70 minutes). Marking the build as aborted. And the build locally passes fine on Windows 7. @Andrey, Can you try on linux locally? (In reply to Sarika Sinha from comment #27) > And the build locally passes fine on Windows 7. > @Andrey, > Can you try on linux locally? I observe lot of NPE's on Linux if running the code in debugger. I've highlighted the locations of those NPE's in the gerrit. Creation and closing of the debug hover is the problem. No idea why it does not fail on Windows. Gerrit change https://git.eclipse.org/r/135927 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=eba1bf7d0a18f60fa943ddc8fde2ab860eb09380 (In reply to Andrey Loskutov from comment #10) > I must confess, this way how it is implemented right now it is more > distracting than helping, and I think this must be fixed or reverted. I'm still fighting with the current state. It is not usable from my opinion. > My biggest issue is that it seems to automatically remember all *selected* > values and adds them to the "proposal" list. OK, now it "only" remembers if user *clicked/focused* the text area. Still it makes no sense to do this "on click", it should be "on save". > So while a short debugging > session just selecting variables fills the list with nonsense "proposals". So for example if one wants to select and copy/paste some value from ""this" referenced from" entry, the history automatically remembers the noncense value of "72 references: ...". > After that, one looses an overview *what* is the *actual* value if one > selects something in the list. The irritating thing is also, that after using combo to select one element, the same selected value is shown twice: once in the combo and once in the details area below. It makes no sense. > Second: if the variables are on the right stack, the combo and the proposal > list is to narrow to be useful, can't be resized and so can't be fully read > / used (if it would contain real expressions). At least on Windows this proposal is scrollable now, but still can't be resized. This is unusable. > Third: it allows to select those nonsense values to absolutely unrelated > variables of different types and does not show that the value is actually > not applied yet. There is no indication that the text in the detail pane is > need to be "saved" first. I know this was like this before, but now it is > really confusing. Still there. I will attach some pictures with comments inside where I found strange results. Beside the already mentioned combo limitations (size can't be changed, to narrow, too alien), I have also problems to understand how the entry list and "global history" are computed: - The current value is always shown as "previously selected" in the list. - Not sure what "Global history" is doing - it just contains some *but not all* previously selected entries. What is the filter criteria? - "Global history" contains same elements multiple times. In general, I think this can be fixed by: 1) Remove combo box and use the "light bulb" + content assist instead. Combo is the wrong UI approach here and causes major usability issues. 2) Restrict this "light bulb" to work only with *really* editable entries. So do not offer this on "references" or "no method return value" elements. 3) Do not remember on selection or focus events, remember on "apply". Only if user actively changes the value, we know it was an expression worth to be remembered. Just focusing/selecting something is not an indicabtion the value is an expression entered/changed by user. 4) Remembered elements should be filtered to not have duplicates. 5) Rework (and may be rename / hide) the "Global History". It is not clear why there should be a difference between "local" history and "global", and what this "global" is good for. Offer a way to cleanup this history from nonsense values, or restrict the "memory" to 10 last elements, so that the older will just disappear some time. Created attachment 277393 [details]
combo issues
Created attachment 277394 [details]
works with not editable values
Created attachment 277395 [details]
shows some recently used selection in unrelated context
I will go through the comments and get back within couple of days. As far as I understood - 1. Resizing problem is no different from breakpoints view. 2. I will investigate on Global history 3. We can add the original value in the list if required - intention was to show the values which user had added. I looked at this. The general direction is OK. Let's keep in mind that the introduced combo is not new. It is exactly the same that we have since 8 years in the Breakpoints view. I agree to the following issues that we have in both views and they should be fixed for both at the same time: - Can't resize the combo value window (new size should be persisted) ==> bug 544232. - Should not be able to choose the global history entry (like in Open Type) ==> bug 544234. Andrey, the global history is what's shared between open Variables/Breakpoints views. If it is not correct, a separate bug with steps should be filed. Some bugs I found in the Variables view: - "for debugging" should be removed from the combo text. - On save (Ctrl+S) it should reset the combo text to <Choose.... (like in the Breakpoints view). - The history also seems to remember things that aren't entered by the user. Here's one example: 1. Start new workspace and paste the following into the Package Explorer: public class T { public static void main(String[] args) { int i= 0; System.out.println(i++); System.out.println(i++); System.out.println(++i); } } 2. Set a breakpoint on the secons sysout 3. Start debugging and allow perspective switch 4. Click on 'i' in the Variables view 5. Click on 'args' in the Variables view ==> detail formatter error is shown and remembered in the history. Filed unrelated bug report: Bug 544228: Variables view: content assist hint (light bulb) missing (In reply to Dani Megert from comment #35) > - The history also seems to remember things that aren't entered by the user. > Here's one example: > ... Sorry, that example is not enough to reproduce the problem reliably, but I was able to see the issue again today and also Sarika could see it. She will post better steps. Global History - It stores the history across all variables (breakpoints have a separate global history) I think the main problem is that right now it adds the history on "FocusLost" which is adding unintended items in the drop down. Conditional breakpoint there is no default string coming from any where so save on focus lost doesn't create problem. Points to consider - 1. Like Ctrl + S, provide another shortcut and context menu to add items to the combo. 2. We can not rely on user's typing as user can select a value from Global history. 3. We can check with the original value of the variable and save only if the value has changed @Dani, @Andrey Let me know your thoughts ! (In reply to Sarika Sinha from comment #37) > Global History - It stores the history across all variables (breakpoints > have a separate global history) > > I think the main problem is that right now it adds the history on > "FocusLost" which is adding unintended items in the drop down. Conditional > breakpoint there is no default string coming from any where so save on focus > lost doesn't create problem. Points to consider - > 1. Like Ctrl + S, provide another shortcut and context menu to add items to > the combo. Why not use Ctrl+S as a trigger for this? It is exact the time where user decided to apply some change. Or is the code shared between breakpoint edit dialog and variables view? I can't remember Ctrl+S works in the breakpoint edit dialog? If this is the problem, we should be able to know in which context we are right now and decide. > 2. We can not rely on user's typing as user can select a value from Global > history. I didn't get this. > 3. We can check with the original value of the variable and save only if the > value has changed This makes sense in any case. > We can not rely on user's typing as user can select a value from Global
> history.
This was for the case if we wanted to add the values only if user typed a new value, but a user can choose a new value from the global history ( value of different variable)
(In reply to Sarika Sinha from comment #37) > Global History - It stores the history across all variables (breakpoints > have a separate global history) You mean "Variables views", right? (In reply to Dani Megert from comment #40) > (In reply to Sarika Sinha from comment #37) > > Global History - It stores the history across all variables (breakpoints > > have a separate global history) > You mean "Variables views", right? Yes, Variables view has its own Global history. Breakpoints view Condition history has it's own Global History. If we add the original value of the variable to the combo box, then the error message like detail formatter error will also be added, and the list will lose the purpose of remembering the expression used by the user previously. If we use Ctrl +S, it will remember the expression used to assign the value and not the expressions which were executed at that moment for debugging. (In reply to Sarika Sinha from comment #42) > If we add the original value of the variable to the combo box, then the > error message like detail formatter error will also be added, and the list > will lose the purpose of remembering the expression used by the user > previously. > > If we use Ctrl +S, it will remember the expression used to assign the value > and not the expressions which were executed at that moment for debugging. I must confess, I've misunderstood the original comment 0 and now after re-reading this again I see that the use case is different from what I had in mind. So not only "assigned" values should be remembered, but all possible evaluations? So user can write something inside the Variables and run evaluation on it, *without* saving, and this should be remembered? Why? For this use case we have "Debug Shell" view which remembers all the expressions. But anyway, if we want do this, shouldn't we ask evaluation engine for the list of recent evaluations? Something like List<String> IEvaluationEngine.getRecentEvaluations()? Relying on Ctrl+S in the standard scenario is not practical. The user doesn't even know that this is available. What we have to do is that we don't remember values which haven't been entered by the user, like the error message. Also, I've seen issues with the global history when multiple Variables views are open and used. New Gerrit change created: https://git.eclipse.org/r/136743 Gerrit change https://git.eclipse.org/r/136743 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=0072e827e8885d31e1423a412aa140ade51d9316 (In reply to Eclipse Genie from comment #45) > New Gerrit change created: https://git.eclipse.org/r/136743 This takes care of - the formatter error or any unwanted items in the history. - "For debugging" has been removed - history is consistent with more than 1 variables Please try in a new workspace or remove dialog_settings.xml from o.e.jdt.debug.ui metadata . Note - If there is no local history, only global history items are shown with the label "Global History" (In reply to Andrey Loskutov from comment #43) > (In reply to Sarika Sinha from comment #42) > > If we add the original value of the variable to the combo box, then the > > error message like detail formatter error will also be added, and the list > > will lose the purpose of remembering the expression used by the user > > previously. > > > > If we use Ctrl +S, it will remember the expression used to assign the value > > and not the expressions which were executed at that moment for debugging. > > I must confess, I've misunderstood the original comment 0 and now after > re-reading this again I see that the use case is different from what I had > in mind. So not only "assigned" values should be remembered, but all > possible evaluations? > > So user can write something inside the Variables and run evaluation on it, > *without* saving, and this should be remembered? Why? For this use case we > have "Debug Shell" view which remembers all the expressions. > > But anyway, if we want do this, shouldn't we ask evaluation engine for the > list of recent evaluations? > Something like List<String> IEvaluationEngine.getRecentEvaluations()? This bug was prioritized based on the survey related to debugging experience in 2016. Something like List<String> IEvaluationEngine.getRecentEvaluations() will give the expressions evaluated across - inspect & display in editor, debug shell or any where. New Gerrit change created: https://git.eclipse.org/r/136889 Gerrit change https://git.eclipse.org/r/136889 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=e65de862b50bfb29d6e56c23a50522a1e1c4f3ca Verified using Eclipse SDK Version: 2019-03 (4.11) Build id: I20190218-1800 New Gerrit change created: https://git.eclipse.org/r/137190 Gerrit change https://git.eclipse.org/r/137190 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=5d4dbe6134b1c9b3ed063c4d2f855419ca684e7a N&N added. I still have only garbage in the combo box and I have *nothing* evaluated at all. I will attach a picture showing the combo as it was after 10 minutes of debugging, without a single evaluation. I honestly would prefer to revert this change for M3 because I haven't seen anything except confusion so far from trying to use it. Created attachment 277624 [details]
Unusable combo with meaningless proposals
Are you using a new workspace or deleted the file mentioned earlier? As I said the old workspace might have got cluttered. Unless you have typed that in detail pane, it will not show up in the combo box. (In reply to Sarika Sinha from comment #57) > Unless you have typed that in detail pane, it will not show up in the combo > box. I haven't typed in the Variables view, only used upper part to select variables to see their values in the details part. The values in the combo seem to be those which I've selected during the debug session. Only thing I remember was that I've used "Display" in the Java editor to see values, but never in the details part of the Variables view. Andrey, please try to provide reproducible steps. Created attachment 277632 [details] Another attempt to use it (In reply to Dani Megert from comment #59) > Andrey, please try to provide reproducible steps. Just my current debugging session: I've put a breakpoint at line 62 in org.eclipse.jdt.launching.sourcelookup.containers.JavaSourcePathComputer.computeSourceContainers(ILaunchConfiguration, IProgressMonitor) and start debugger from debuggee (I'm validating patch https://git.eclipse.org/r/137257). Before debugger returns from the method, inspect the content of the getSourceContainers() return - this is an array. Select an array entry there. *Click once in the details pane*, don't select anything, just click. Select another entry. Click in the combo: it shows previously selected value. Funny enough, combo shows *different* proposals list on different selection *on the same* breakpoint/frame. I can't explain why and have no idea why proposals should differ depending on the selected value. Means: clicking inside the details pane is enough to add the entry to the combo. This should not happen. (In reply to Andrey Loskutov from comment #60) > Means: clicking inside the details pane is enough to add the entry to the > combo. That doesn't happen for me using eclipse-SDK-I20190219-1800-win32-x86_64. As said, would be good to have reproducible steps, starting with a new workspace and best captured in a new bug report. (In reply to Dani Megert from comment #61) > (In reply to Andrey Loskutov from comment #60) > > Means: clicking inside the details pane is enough to add the entry to the > > combo. > That doesn't happen for me using eclipse-SDK-I20190219-1800-win32-x86_64. Maybe you're not on Windows and it is an OS specific issue? I think it happens only for strings. for String the previous value comes as "oldValue" as it is shown in the Variables view Value column and that is why it is treating it as different to oldValue. (In reply to Sarika Sinha from comment #63) > I think it happens only for strings. > > for String the previous value comes as "oldValue" as it is shown in the > Variables view Value column and that is why it is treating it as different > to oldValue. Indeed with strings I see the issue. It also resulted in [] ending up in the history. And then [] dropped out again. So, this definitely needs a fix for 4.11. I leave it to you Sarika, whether you reopen this bug or file a new one. New Gerrit change created: https://git.eclipse.org/r/137295 I have the fix, so will fix it by this bug itself. Thanks Andrey for providing the steps and attachment! (In reply to Sarika Sinha from comment #66) > I have the fix, so will fix it by this bug itself. Reopening then. > Thanks Andrey for providing the steps and attachment! Yes, thanks! Gerrit change https://git.eclipse.org/r/137295 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=e0d2c6bdf3adc29a192c52aec550ef0d9b468ab1 (In reply to Eclipse Genie from comment #68) > Gerrit change https://git.eclipse.org/r/137295 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=e0d2c6bdf3adc29a192c52aec550ef0d9b468ab1 > This fix is not complete. Still stores e.g. String arrays, e.g. [1, 2]. (In reply to Dani Megert from comment #69) > (In reply to Eclipse Genie from comment #68) > > Gerrit change https://git.eclipse.org/r/137295 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=e0d2c6bdf3adc29a192c52aec550ef0d9b468ab1 > > > This fix is not complete. Still stores e.g. String arrays, e.g. [1, 2]. Same problem with int arrays and probably all other arrays. I don't understand the latest fix. I would expect a fix that ensures that there is never a "save" just when clicking. We must only save when the user actively types in the details pane, as you said yourself in comment 57. New Gerrit change created: https://git.eclipse.org/r/137329 (In reply to Dani Megert from comment #71) > I don't understand the latest fix. I would expect a fix that ensures that > there is never a "save" just when clicking. We must only save when the user > actively types in the details pane, as you said yourself in comment 57. I had tried before but ModifyListener sends event for each letter modification and we don't want the bits and pieces in the combo values. but now I think it looks ok with the combination of modify and my previous solution. New gerrit take care of adding to the list only if modified. (In reply to Sarika Sinha from comment #73) > (In reply to Dani Megert from comment #71) > > I don't understand the latest fix. I would expect a fix that ensures that > > there is never a "save" just when clicking. We must only save when the user > > actively types in the details pane, as you said yourself in comment 57. > > I had tried before but ModifyListener sends event for each letter > modification and we don't want the bits and pieces in the combo values. > but now I think it looks ok with the combination of modify and my previous > solution. > > New gerrit take care of adding to the list only if modified. Tried it out and seems to work so far. Andrey, can you also test the proposed fix? Thanks. (In reply to Dani Megert from comment #75) > Andrey, can you also test the proposed fix? Thanks. You probably mean patch https://git.eclipse.org/r/137329. The good news is that it seem not to add "unrelated" selections to the history, almost. I've again managed to push some not edited values into the history, but only once, can't reproduce yet. So I assume the biggest issue I had before is almost gone. Still there are number of usability issues I personally have with the feature. 1) I see that sometimes selected combo values aren't applied. I have "a", "b", "c" elements in the combo. Select "a" in the combo => text is set. *Delete* text from the details pane and select "a" again => no effect at all. Looks like the value is applied only if the combo selection changes. It looks also that at some point in time the inserted values do NOT match selected, at least it happened to me twice, but I could not understand the preconditions for that. I've also managed to have same proposals multiple times in the "local" history. Here too, no idea how to reproduce. 2) I still can't really understand why do we have global/local history differentiation, especially because the current implementation leaks all "remembered" variables for the entire session to implement this. 3) The global history confuses me - it always contains "local" elements twice (once in the "local" and once in the "global" history). It would probably make sense if one would run evaluation on different variables multiple times, but then the "global" history should not contain "local" entries. So I don't see a value to have "local" history if it is duplicated in the global one. 4) Most "remembered" "global" expressions can't be evaluated in the context of another variables/frames (an error appears trying to do so). So the usefulness of the global history is questionable in context of Variables view. This is different to the Breakpoints view, where we don't have "current" frame and don't evaluate immediately. 5) 3+4 together requirements are conflicting. Also the "global" history is not shared between Variables and Breakpoints views, so it is not "global" in that sense. 6) If I select "a" in the combo, I see the "a" in the details area and in the combo, the text "<Choose a previously selected expression>" disappears. It looks strange at least, to see "a" twice. 7) After selection of the value the value is not applied. So selecting a value, resize the view => the new value is replaced with the real value of the variable. This is surprising at least. 8) Related to the previous point: if I change the value of conditional breakpoint condition, the Breakpoints view shows asterisk "*" to indicate a "save" is needed. This is not the case in the Variables view. It is not introduced in this bug, but it is just another inconsistency we will expose with the combo. 9) Long values make the combo more or less unusable in the default Variables view position (right stack next to the editor area). Selection of values from such combo is not nice UI experience. 10) On save (Ctrl+S) the combo text is not reset to <Choose... (like in the Breakpoints view). Summary: I'm not convinced how UI works. I don't like the combo. I don't understand the use model (in the context of Variables view, especially with the differentiation of global/local history). I believe the entire feature is not usable for the average user and also does not provide a value for most of the users. It just doesn't feel "right" and makes the debugger UI more complex. I personally believe that we should not ship this feature to the end users in 4.11 and remove (or hide) in RC1. (In reply to Andrey Loskutov from comment #76) I did not yet go through your items, but are you aware that we have exactly the same in the Breakpoints view since 2011? And we did not get any complaints. We would probably have to work on both views together. (In reply to Dani Megert from comment #77) > (In reply to Andrey Loskutov from comment #76) > I did not yet go through your items, but are you aware that we have exactly > the same in the Breakpoints view since 2011? The code is not same, definitely not, especially the implementation of the local/global history. > And we did not get any > complaints. We would probably have to work on both views together. The one works. Another doesn't. The views have also different use model. Breakpoints view is "detached" from any context, Variables view is tightly connected to the current selected frame. > especially because the current implementation leaks all "remembered" variables for the entire session to implement this.
Sarika, this is a real concern.
The Local and Global history implementation is exactly same as the Breakpoints, but yes the context is different.
In breakpoints, local means expressions used for that particular breakpoint and global means expressions used in other breakpoints.
In Variables, local means expressions used for the selected variable and the global means for other variables.
@Andrey
>>2) I still can't really understand why do we have global/local history >>differentiation, especially because the current implementation leaks all >>"remembered" variables for the entire session to implement this.
What do you mean by this ?
Local history has a maximum length of 10 for Breakpoints and Variables. Both have the same persistence strategy for Breakpoint/Variables and their locla values. We can do away with Local/Global differentiation in totality from both the views if we want. (In reply to Sarika Sinha from comment #80) > @Andrey > >>2) I still can't really understand why do we have global/local history >>differentiation, especially because the current implementation leaks all >>"remembered" variables for the entire session to implement this. > > What do you mean by this ? https://git.eclipse.org/r/#/c/137329/2/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/variables/JavaVariablesDetailPane.java@218 (In reply to Sarika Sinha from comment #81) > Local history has a maximum length of 10 for Breakpoints and Variables > We can do away with Local/Global differentiation in totality from both the > views if we want. This would fix some of the mentioned issues. I believe that "global" history of size 20 would be enough. I will be going ahead in dropping local history for Variables view. There will be only a global history with 20 items at max. All other things will remain the same. (In reply to Sarika Sinha from comment #83) > I will be going ahead in dropping local history for Variables view. There > will be only a global history with 20 items at max. All other things will > remain the same. Gerrit is updated after removing Local History. Gerrit change https://git.eclipse.org/r/137329 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=87084f85244cdf32f0e6594cc78d455685b0380c Verified using Eclipse SDK Version: 2019-03 (4.11) Build id: I20190226-1800 |