dap: refactor how step in/step out works #3325
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c315aa3 to
834d9a4
Compare
ktock
reviewed
Jul 22, 2025
dap/thread.go
Outdated
Comment on lines
428
to
430
| // Iterate from the first step to find the one | ||
| // we failed on. | ||
| result = t.entrypoint | ||
| for result != nil { | ||
| next := result.in | ||
| if next != nil && next.dgst == errDgst { | ||
| break | ||
| } | ||
| result = next | ||
| } |
There was a problem hiding this comment.
nit: no need to return an error if not found?
There was a problem hiding this comment.
I don't think so. The main reason for returning the step is to be able to include the correct location information in the stack trace. I don't think there's really an effective way to return an error here. It might be useful to add some more debugging statements and a way to enable debugging for the dap adapter in the future.
834d9a4 to
d12213f
Compare
In the repl context, we will now return the error instead of directly printing it. We also suppress reporting errors from cobra. The logic flow has also been changed to prevent returning errors from cobra unless there was something related to the command line invocation so usage will only be printed when a command was typed wrong and it will not show up for every error. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Change how breakpoints and stepping works. These now work more how you would expect another programming language to work. Breakpoints happen before the step has been invoked rather than after which means you can inspect the state before the command runs. This has the advantage of being more intuitive for someone familiar with other debuggers. The negative is that you can't run to after a certain step as easily as you could before. Instead, you would run to that stage and then use next to go to the step directly afterwards. Step in and out also now have different behaviors. When a step has multiple inputs, the inputs of non-zero index are considered like "function calls". The most common cause of this is to use `COPY --from` or a bind mount. Stepping into these will cause it to jump to the beginning of the call chain for that branch. Using step out will exit back to the location where step in was used. This change also makes it so some steps may be invoked multiple times in the callgraph if multiple steps depend on them. The reused steps will still be cached, but you may end up stepping through more lines than the previous implementation. Stack traces now represent where these step in and step out areas happen rather than the previous steps. This can help you know from where a certain step is being used. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
d12213f to
1e3c447
Compare
tonistiigi
approved these changes
Jul 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change how breakpoints and stepping works. These now work more how you
would expect another programming language to work. Breakpoints happen
before the step has been invoked rather than after which means you can
inspect the state before the command runs.
This has the advantage of being more intuitive for someone familiar with
other debuggers. The negative is that you can't run to after a certain
step as easily as you could before. Instead, you would run to that stage
and then use next to go to the step directly afterwards.
Step in and out also now have different behaviors. When a step has
multiple inputs, the inputs of non-zero index are considered like
"function calls". The most common cause of this is to use
COPY --fromor a bind mount. Stepping into these will cause it to jump to the
beginning of the call chain for that branch. Using step out will exit
back to the location where step in was used.
This change also makes it so some steps may be invoked multiple times in
the callgraph if multiple steps depend on them. The reused steps will
still be cached, but you may end up stepping through more lines than the
previous implementation.
Stack traces now represent where these step in and step out areas
happen rather than the previous steps. This can help you know from where
a certain step is being used.