Removing UIMnagaer dependencies from the debugger infrastructure#16015
Conversation
Instead, it asks all debugger if they can be opened, and debuggers now by default can open only if we're in graphical mode. Debuggers that can open in headless (e.g.., loggers) have to redefine `handleDebugRequest:` and return an appropriate boolean.
Debugger using the core model should not care about the UI backend.
guillep
left a comment
There was a problem hiding this comment.
Certainly much cleaner!
I'll put the PR as draft to avoid accidental merges :)
| self spawnNewUIProcessIfNecessary: aDebugRequest. | ||
| self ensureExceptionIn: aDebugRequest debugSession. | ||
|
|
||
| "Schedule the debugger opening in a deferred UI message to address redraw problems after opening a debugger e.g. from the testrunner." | ||
| debuggerOpeningStrategy := self debuggerSelectionStrategy. | ||
| self defaultUIManager defer: [ | ||
| debuggerOpeningStrategy openDebuggerForSession: | ||
| aDebugRequest debugSession ]. |
There was a problem hiding this comment.
Could you comment somewhere in the PR where is the code replacing this functionality? Is it in Spec? Somewhere else? is it WIP?
There was a problem hiding this comment.
Sorry, it is there pharo-spec/NewTools#667
The spec debugger asks its application for the used backend, which in turn gives back a debug session specialized for that backend (Morphic or Gtk).
The specialized debug session is now handling the removed code that you pointed out.
It also means that if I want to create a new debugger, and if it should work with Morphic, I also have to ask the backend and the specialized session what to do.
There was a problem hiding this comment.
In other words: Morphic affairs are now Morphic affairs only.
|
This is really good to see some improvements there! |
|
@guillep @StevenCostiou is this ready to go? |
|
This is too complicated and now we're frozen... |
|
I moved it to P13 and relaucnhed the tests |
|
Thanks! |
|
All the tests are passing on windows so I will merge it. |
|
Thanks! |
|
Thanks for the rollback @jecisc Looks like more work is required from @StevenCostiou This change froze the image when you already have put a simple halt into your code or call a non-existing message on a class (leading to a DNU). |
|
We were missing some code from NewTools. This should work now and the change is again present in the image |
Not really, it was written on the PR description: do not merge! |
Debuggers are now responsible to handle specific backend requirements:
The PR alone kills the spec debugger.
The debugger can only work after a dedicated PR taking into account the modifications of this PR.
This PR should: