interp: implement ERR and EXIT trap #650
Closed
Conversation
This file contains 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
Make room for the ERR and EXIT callbacks.
zimbatm
force-pushed
the
trap-err-exit
branch
from
661d279
to
b7acdbb
Compare
|
I'll still be quite AFK tomorrow, but after that I'll start being around again. Let me know if you want some help and I'm happy to do that - possibly over Slack, since it's more instantaneous :) |
|
Ok, I think this is not over yet. I'm also in holiday mode but I might jump into Slack at some point. |
|
@zimbatm do you want to continue with this? I'm aiming for a v3.3.0 release by the end of February, and I think including the features/fixes needed by direnv would be nice :) I'm happy to take over this PR if you don't have spare cycles at the moment, but I thought I'd ask before I do. |
|
Go for it. I did the mini spike and now it has swapped out of priority. Thanks a lot for all the wonderful work you're doing. |
mvdan
added a commit
that referenced
this pull request
Feb 16, 2021
Thanks to Jonas Chevalier for implementing the bulk of this on #650. His changes were mainly implementing the "trap" builtin itself, recording the trap callbacks in new fields, and refactoring the exit logic to allow calling those callbacks. And, of course, the tests. My changes on top extend the tests a bit, make Runner.stmtSync call the ERR trap callback when a simple command fails, and make Runner.Run call the EXIT trap callback when done interpreting an entire *File. That last change is backwards compatible, since the only noticeable effect should be that Run on a *File can call EXIT trap callbacks. Those were not supported before, so no existing program should break. Finally, note that we add a test case that would make the Runner get stuck in a callback loop without the handlingTrap boolean field.
|
I've finished up your patch in a new PR: #666 A review or a confirmation that it works for you would be welcome. If you need anything else implemented, an option is also to review and merge that PR and send follow-up ones :) |
mvdan
added a commit
that referenced
this pull request
Feb 16, 2021
Thanks to Jonas Chevalier for implementing the bulk of this on #650. His changes were mainly implementing the "trap" builtin itself, recording the trap callbacks in new fields, and refactoring the exit logic to allow calling those callbacks. And, of course, the tests. My changes on top extend the tests a bit, make Runner.stmtSync call the ERR trap callback when a simple command fails, and make Runner.Run call the EXIT trap callback when done interpreting an entire *File. That last change is backwards compatible, since the only noticeable effect should be that Run on a *File can call EXIT trap callbacks. Those were not supported before, so no existing program should break. Finally, note that we add a test case that would make the Runner get stuck in a callback loop without the handlingTrap boolean field. Fixes #635.
|
I'm merging that PR for now, and I'll close this one too - any testing or thoughts still welcome, of course. I just want to start preparing for an upcoming release :) |
mvdan
added a commit
that referenced
this pull request
Feb 24, 2021
Thanks to Jonas Chevalier for implementing the bulk of this on #650. His changes were mainly implementing the "trap" builtin itself, recording the trap callbacks in new fields, and refactoring the exit logic to allow calling those callbacks. And, of course, the tests. My changes on top extend the tests a bit, make Runner.stmtSync call the ERR trap callback when a simple command fails, and make Runner.Run call the EXIT trap callback when done interpreting an entire *File. That last change is backwards compatible, since the only noticeable effect should be that Run on a *File can call EXIT trap callbacks. Those were not supported before, so no existing program should break. Finally, note that we add a test case that would make the Runner get stuck in a callback loop without the handlingTrap boolean field. Fixes #635.
mvdan
added a commit
that referenced
this pull request
Feb 24, 2021
Thanks to Jonas Chevalier for implementing the bulk of this on #650. His changes were mainly implementing the "trap" builtin itself, recording the trap callbacks in new fields, and refactoring the exit logic to allow calling those callbacks. And, of course, the tests. My changes on top extend the tests a bit, make Runner.stmtSync call the ERR trap callback when a simple command fails, and make Runner.Run call the EXIT trap callback when done interpreting an entire *File. That last change is backwards compatible, since the only noticeable effect should be that Run on a *File can call EXIT trap callbacks. Those were not supported before, so no existing program should break. Finally, note that we add a test case that would make the Runner get stuck in a callback loop without the handlingTrap boolean field. Fixes #635.
|
Merged; closing. |
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.
This is still a WIP. The ERR and EXIT callbacks are not really at the right
place I think, they seem to fire on every shell prompt in gosh.