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

onExit callback should be typed to number #162

Closed
hexa00 opened this issue Dec 5, 2017 · 7 comments
Closed

onExit callback should be typed to number #162

hexa00 opened this issue Dec 5, 2017 · 7 comments

Comments

@hexa00
Copy link

hexa00 commented Dec 5, 2017

https://github.com/Tyriar/node-pty/blob/e9c790871b55c8fe9920409f59fb12360adda999/src/unixTerminal.ts#L73

This line cast away the types returned by:

https://github.com/Tyriar/node-pty/blob/e9c790871b55c8fe9920409f59fb12360adda999/src/unix/pty.cc#L482

Thus if one tries:

exitCallback (code, signal) {

  if (signal === '2') {
   // Do something
  }
}

The test will fail, note that one may get the impression that signal is a string since it is in nodejs API see: https://nodejs.org/dist/latest-v8.x/docs/api/child_process.html#child_process_event_exit

Typing the callback would avoid that issue I think.

@hexa00
Copy link
Author

hexa00 commented Dec 5, 2017

Actually maybe it won't fix the issue since the callback signature isn't carried though emit but it would give the right hint to a dev looking at the code...

Tyriar added a commit that referenced this issue Dec 11, 2017
@Tyriar
Copy link
Member

Tyriar commented Dec 11, 2017

I changed the code but you're right it will only give a hint when looking at the library code. What you want is possible by explicitly typing out the emit functions, but it will need the handwritten typing file to be done first #154

@hexa00
Copy link
Author

hexa00 commented Dec 11, 2017

@Tyriar thx, nice I didn't know this was possible, looking forward to it :)

@Tyriar
Copy link
Member

Tyriar commented Dec 11, 2017

@hexa00
Copy link
Author

hexa00 commented Dec 11, 2017

Right, nice :)

@Tyriar
Copy link
Member

Tyriar commented Oct 21, 2021

This is done:

readonly onExit: IEvent<{ exitCode: number, signal?: number }>;

@Tyriar Tyriar closed this as completed Oct 21, 2021
@hexa00
Copy link
Author

hexa00 commented Oct 21, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants