Community
Participate
Working Groups
Build Identifier: M20100909-0800 Some debug backends can't handle the case where the user doesn't enter any text and pushes "OK". It would be better to simply disable the "OK" button if no expression has been provided. Reproducible: Always Steps to Reproduce: 1.Debug a program 2.Open the Memory view 3.Click the green + "Add Memory Monitor" 4.Make sure the Combo box has the empty string in it and push "OK" depending on the debugger backend, you'll see different behaviour
I suggest making the dialog implement VerifyListener, and add itself as a listener for the Combo. e.g.: @Override public void verifyText(VerifyEvent e) { setOKButtonEnabled(e.text); } /** * Based on the expression, the OK button will either be enabled or disabled. * @param expression */ private void setOKButtonEnabled(String expression) { updateStatus(new Status((expression == null || expression.isEmpty()) ? IStatus.ERROR : IStatus.OK), DebugPlugin.getPluginID(), null)); } You will also want to call setOKButtonEnabled() with the text in the Combo box before the dialog is displayed.
Created attachment 182839 [details] proposed fix The patch adds the dialog as a verify listener and updates the existing updateOKButtonState() method to work properly
> The patch adds the dialog as a verify listener Why is this needed? I think the modify listener should be enough once updateOKButtonState() is correctly implemented. Also, if we think about backporting this in the future, I would like to see a minimal patch that only changes what's needed to fix this bug.
I am not sure we want to do this. The OK button was enabled as designed. There was a requirement to allow user to enter empty expression. The debug adapter needs to decide what that means. I remember in some cases, we need to open memory at address 0x0. https://bugs.eclipse.org/bugs/show_bug.cgi?id=98995
Created attachment 183014 [details] min patch A 'minimized' patch.
I must agree with Samantha on this: there is currently no API that immediately tells whether the expression is valid hence we cannot exclude the empty string. IMemoryBlockRetrieval[Extension].get[Extended]MemoryBlock(*) probably has to throw a DebugException if it can't handle the empty (or any other invalid expression). This should then be presented in the UI. Looking at the code this already seems to happen. Sean, can you give more details what's actually happening on your side and why it is a problem for you?
Based on what Sam's dug up, it sounds like we shouldn't be making this change. What we found is that some debug back ends don't handle an empty expression properly. Since, in some production conditions, the engine is difficult to change, I thought maybe to prevent the requets ever being made. The real problem is with the back end, and since the original design explicitly allowed an empty expression, it seems like the back ends with the problem need to be fixed to handle one, and users suck with back levels will just have to be careful ...
Thanks for the quick answer Sean.
