Community
Participate
Working Groups
Build Identifier: 3.7.0 I20110428-0848 In the expressions view, it is possible to click on an expression and edit it. If the user puts in a blank expression, the expression should be removed instead of leaving a blank expression inside the view. The reason is to reflect the same logic in ExpressionManagerContentProvider.AddNewExpressionElement.modify that checks for an empty string and allows only non-empty string being added to expression manager. Reproducible: Always Steps to Reproduce: 1.Launch eclispe without any debugger. 2. Add an non-empty expression 3. Edit the expression from (2) to either make it an empty string or a string full with blanks only. I expect step 3 will remove the expression rather than the view keeps the expression with blanks or keeps the empty string.
Created attachment 195754 [details] patch Please review this patch and include it in 3.7 RC2 or 3.7.0.
IMO the safer thing to do would be to revert the expression back to what it was before the edit, rather than removing it. I don't know if the bug is severe enough to target for RC2, I'd rather leave it for 3.8, or 3.7.1. Opinions?
+1 for 3.8, like you say, its not critical or blocking anyone.
(In reply to comment #3) > +1 for 3.8, like you say, its not critical or blocking anyone. (In reply to comment #2) > IMO the safer thing to do would be to revert the expression back to what it was > before the edit, rather than removing it. > I don't know if the bug is severe enough to target for RC2, I'd rather leave it > for 3.8, or 3.7.1. > Opinions? I double-verify that, a) if user press ESC button while modifying, the modify method is not called. b) The only time for end-user to apply the change is when ENTER button is pressed and so the modify method is invoked. Thus, I don't think it should revert to the previous string because the user does not intend to cancel the change -- the user intends to apply the change.
(In reply to comment #3) > +1 for 3.8, like you say, its not critical or blocking anyone. This bug is to reflect the need of consistency of ExpressionManagerContentProvider.AddNewExpressionElement.modify. Also, if an empty string is left in the view while there are no debuggers, its impact depends on how individual debuggers handle it when they become active, for e.g., - java debugger handles it by always quoting each of the variables; - cdi gdb shows an error [see cdi_gdb screenshot]; - dsf gdb has a blank row (no name, no errors) and the "Add new expression" is shifted below the row [see dsf_gdb screenshot]. When dsf gdb becomes active and user clicks on the empty string, the expression gets removed, which is a result of Bug 233111 addressed by Pawel. This is not a fatal defect. +1 for 4.7.1.
Created attachment 196004 [details] cdi gdb screenshot
Created attachment 196007 [details] dsf gdb screenshot
(In reply to comment #5) > When dsf gdb becomes active and user clicks on the empty string, the expression > gets removed, which is a result of Bug 233111 addressed by Pawel. I guess I contradicted myself :-o It's funny how much more conservative I think in context of the platform project. But stepping back to my former self, I guess removing the expression is more useful.
Hello Pawel, Do you think you can review this patch and get it committed to 3.8 whichever the earliest milestone? Thanks, Winnie (In reply to comment #8) > (In reply to comment #5) > > When dsf gdb becomes active and user clicks on the empty string, the expression > > gets removed, which is a result of Bug 233111 addressed by Pawel. > I guess I contradicted myself :-o It's funny how much more conservative I > think in context of the platform project. But stepping back to my former self, > I guess removing the expression is more useful.
(In reply to comment #9) > Hello Pawel, > > Do you think you can review this patch and get it committed to 3.8 whichever > the earliest milestone? Sure. Thank you for bring it up.
I pushed the fix. Thank you for the contribution. http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=6f4ad90bdd669f6e4a0b1313240a75c5294757b8
