Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Debugger.pause() and Debugger.isExecuting() methods. #153

Closed
wants to merge 1 commit into from

Conversation

entlicher
Copy link
Contributor

ExecutionEvent is made disposable. When outside of event handler process, all it's methods should throw IllegalStateException, according to javadoc. Also, I've deprecated prepareContinue() method, since continue is by default.
To add a "pause" functionality (which could be achieved by calling ExecutionEvent.prepareStepInto() any time during language execution, but this is not possible any more), Debugger.pause() method is introduced. Also, Debugger.isExecuting() is added so that we can test whether we can expect that pause() will actually pause anything.
Test is included.

@@ -87,9 +90,11 @@ public Debugger getDebugger() {
* </ul>
*
* @since 0.9
* @deprecated Continue is by default, this method is not necessary.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is necessary to deprecate this method. I know continue is the default action, but having the method allows people to "change their mind". It keeps some symmetry and also advertises that debugger can "continue".

I suggest to integrate the change without this deprecation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you have just a single event handler callback to "change your mind", thus I considered this method unnecessary.
But O.K. if you'd like to keep it, I'll change the code accordingly.

@entlicher entlicher force-pushed the DebuggerPause branch 3 times, most recently from 350d4ba to 957abaf Compare March 30, 2016 14:13
@entlicher
Copy link
Contributor Author

I've updated the pull request and addressed @jtulach comments.
Also, the SLDebugTest.testPause() is made more robust.

@jtulach
Copy link
Contributor

jtulach commented Apr 4, 2016

I am OK with this change from an API and test perspective.

@graalvmbot
Copy link
Collaborator

@graalvmbot
Copy link
Collaborator

  • User Martin Entlicher with email address martin (dot) entlicher (at) oracle (dot) com has not signed Oracle OCA

@graalvmbot
Copy link
Collaborator

  • User Martin Entlicher with email address martin (dot) entlicher (at) oracle (dot) com is clear for merging this pull request

dougxc pushed a commit that referenced this pull request Jul 28, 2016
…tely-test to master

* commit 'af32b3ff953bbe8b176bf37385fe62ec8e883371':
  Make SLTestSuite more flexible
@entlicher
Copy link
Contributor Author

Debugger.pause() is there already, but is replaced with DebuggerSession.suspendNextExecution().

@entlicher entlicher closed this Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants