Bug 201879 - Display and Inspect actions should not change editor selection
Summary: Display and Inspect actions should not change editor selection
Status: CLOSED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: bugday, helpwanted
Depends on:
Blocks:
 
Reported: 2007-08-31 07:12 EDT by Markus Keller CLA
Modified: 2018-10-15 10:20 EDT (History)
4 users (show)

See Also:


Attachments
Patch for 201879 (5.15 KB, patch)
2007-09-30 15:14 EDT, Joern Dinkla CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2007-08-31 07:12:37 EDT
I20070828-0800

The Display and Inspect actions should not change the editor selection. E.g. in the example below, debug to the breakpoint and the evaluate Math.pow(x, 2). The result is shown, but also the selection is cleared and the caret set to column 1 of the line with the current instruction pointer.

This makes it hard to use the Display and Inspect actions repeatedly with the keyboard, since you lose the selection context every time.

public class Try {
	public static void main(String[] args) {
		int x= 1; int y= 2;
		double dist= Math.sqrt(Math.pow(x, 2) + Math.pow(y, 2));
		System.out.println(dist); //breakpoint
	}
}
Comment 1 Michael Rennie CLA 2007-08-31 10:26:45 EDT
it seems that details are recomputed after the action runs, this could be a by-product of the evaluation engine running to perform the display evaluation and the evaluation listener notifying the default detail pane that an evaluation has completed.
Comment 2 Curtis Windatt CLA 2007-08-31 13:18:55 EDT
Marking as a bug day candidate
Comment 3 Joern Dinkla CLA 2007-09-26 17:37:32 EDT
I like to work on this one. Regards, Joern.
Comment 4 Curtis Windatt CLA 2007-09-27 10:05:48 EDT
It's yours.  Have fun :)
Comment 5 Joern Dinkla CLA 2007-09-28 16:08:50 EDT
Hi,

Here are my first results (i didn't have much time for this, but i will work on his one on Sunday again).

The selection is changed when CompilationUnitEditor(AbstractTextEditor).selectAndReveal(int, int) is called 

The stack trace looks as following.

CompilationUnitEditor(AbstractTextEditor).selectAndReveal(int, int) line: 5879	
SourceLookupFacility.positionEditor(ITextEditor, IStackFrame) line: 317	
SourceLookupFacility.display(ISourceLookupResult, IWorkbenchPage) line: 241	
DebugUITools.displaySource(ISourceLookupResult, IWorkbenchPage) line: 775	
StackFrameSourceDisplayAdapter$SourceDisplayJob.runInUIThread(IProgressMonitor) line: 167	
UIJob$1.run() line: 94	
RunnableLock.run() line: 35	
UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 123	
Display.runAsyncMessages(boolean) line: 3212	
Display.readAndDispatch() line: 2956	
Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2389	
Workbench.runUI() line: 2353	

This is called after the code has been run by the ASTEvaluationEngine.

On Sunday I will look for a way to a) prevent this or b) to allow the editor to restore the old selection. CompilationUnitEditor has a RememberedSelection, but it's methods are protected.

Best regards and have a nice weekend,

Joern
Comment 6 Joern Dinkla CLA 2007-09-30 15:14:43 EDT
Created attachment 79457 [details]
Patch for 201879
Comment 7 Joern Dinkla CLA 2007-09-30 15:22:18 EDT
The call to selectAndReveal() is somewhere deep in the code and after some thinking i decided to take route b) and to overwrite the selectionChanged() method. Before the popup is opened the current selection is saved and a boolean flag is set. In selectionChanged() this flag is tested and the selection is restored. This works because the popup window is modal.

I used a boolean flag and the selection. One could only use the selection and test against null, but i once read that this is considered as bad programming style ;-)

Best regards,

Joern
Comment 8 Curtis Windatt CLA 2007-10-01 15:20:14 EDT
Thanks for looking that this Joern.  The problem with your solution is that the selectionChanged() event may not be called immediately after the popup closes.  With your patch I encountered cases where I tried selecting other parts of the editor and suddenly my selection changed to where I last called inspect.

I have created a new fix and put it in HEAD.  I used the same concept, storing the selection and replacing it after the selection.  The replacement now occurs when the popup dialog is closed (in the PopupDialog.close() method).

See PopupDialogAction.java and PopupInspectAction.java
Comment 9 Curtis Windatt CLA 2007-10-01 15:21:03 EDT
Mike, please verify.
Comment 10 Markus Keller CLA 2007-10-02 06:35:34 EDT
The fix works fine when the actions are executed from an editor selection, but the caret is still moved when executed in the Display or Variables view.
Comment 11 Curtis Windatt CLA 2007-10-02 09:47:09 EDT
Inspect in the Display view opens a popup and shouldn't move the caret.  The display commands adds text to the view and the caret is moved to the end of the inserted text, this seems like reasonable behaviour.  In the variables view, running an evaluation could change the value of the variables, so it is necessary to refresh the variables view - resulting in details being recomputed.  I would again say that this is reasonable behaviour.  If you think the behaviour should be changed, please open a new request.
Comment 12 Markus Keller CLA 2007-10-02 10:16:57 EDT
(In reply to comment #11)
Sorry, I was not too clear in comment 10. I agree with the caret movements in the Display and Variable views, but I found that the caret *in the editor* is still moved when I invoke the actions in one of the views.

E.g.
- select an expression in the editor
- Ctrl+Shift+D, Ctrl+Shift+D
- edit the expression and execute it again in the Display view
- F12
=> caret in the editor is at current instruction pointer; selection is lost

Sorry that I didn't realize this earlier, but I think Joern's a) at the end of comment 5 would be the better approach.
Comment 13 Curtis Windatt CLA 2007-10-02 10:29:01 EDT
I agree that (a) is the better solution, and I attempted to fix this in that manner.  I discussed it with Darin and we couldn't think of a way to fix this that wouldn't break a other functionality.  That is why the (b) solution was pursued.

When inspect/display/execute commands are run, an evaluation must take place.  This results in a SUSPEND/EVALUATION event being fired.  The debug view sees the SUSPEND event and refreshes the debug context.  This updates various views including the variables view in case variable values have changed and also results in a source lookup.  The source lookup resets the caret and selection to the currently executing line.
Comment 14 Curtis Windatt CLA 2007-10-02 10:45:21 EDT
I'm going to reopen this and look at it some more.  If we can figure out something that would be useful.
Comment 15 Curtis Windatt CLA 2007-10-09 14:51:47 EDT
Couldn't come up with an alternate solution, leaving this bug open.
Comment 16 Eclipse Genie CLA 2018-10-15 10:20:26 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

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

--
The automated Eclipse Genie.