-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add full stub for Windows signals #13131
Add full stub for Windows signals #13131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Especially moving everything into Crystal::System
. 👍
I only have a couple of comments on improving the wording in docs.
Pinging @lbguilherme to see if this move into |
It seems fine as is, shouldn't affect anything. Actually, with your changes it could be possible to require signal on the prelude for wasm... I can test that later. |
Resolves #7339.
This defines the 6 signals required by the C standard, using their values in MSVC's headers, plus one Windows-exclusive signal,
Signal::BREAK
. The oldSignal::KILL
no longer exists, as it was only a placeholder value needed by the standard library specs to compile, and this PR removes that need. (Most POSIX signals do not exist on Windows, but technically this was already a problem forPWR
,STKFLT
, andUNUSED
on non-Linux systems.)All of
Signal
's methods (#trap
,#ignore
,#reset
) simply raiseNotImplementedError
. This is intentional, as the other platform-independent replacement APIs shall suffice for the moment.