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

Add kill function to spawn object #40

Closed
kentcdodds opened this issue Aug 29, 2016 · 8 comments
Closed

Add kill function to spawn object #40

kentcdodds opened this issue Aug 29, 2016 · 8 comments

Comments

@kentcdodds
Copy link

Originally I wanted to extract this as a separate module, but I think it actually makes most sense to simply add it to cross-spawn itself. Would you be interested? The win32 version of the function would be:

function kill() {
  crossSpawn("taskkill", ["/F", "/T", "/PID", this.pid])
}

And the posix:

const getDescendentProcessInfo = require("ps-tree")

function kill() {
    getDescendentProcessInfo(this.pid, (err, descendent) => {
        if (err) {
            return
        }

        descendent.forEach(({PID: pid}) => {
            try {
                process.kill(pid)
            }
            catch (_err) {
                // ignore.
            }
        })
    })
}

Would you be willing to accept a PR for this?

cc @mysticatea

@satazor
Copy link
Contributor

satazor commented Aug 30, 2016

I think this is out of the scope of this project and should be in its own module.

Thanks anyway.

@satazor satazor closed this as completed Aug 30, 2016
@kentcdodds
Copy link
Author

cross-spawn-with-kill coming soon to an npm near you :)
cc @mysticatea

@hemanth
Copy link

hemanth commented Aug 31, 2016

heh heh I came here exactly for this!

@kentcdodds
Copy link
Author

It's a thing now :) https://github.com/kentcdodds/cross-spawn-with-kill

@satazor
Copy link
Contributor

satazor commented Sep 3, 2016

Cool

@Igmat
Copy link

Igmat commented Oct 27, 2016

@kentcdodds, thanks a lot for your package - it saves my day.
@satazor, are you sure that kill() for child processes is out of scope for your package? I've spent two days trying to figure out why my integration test doesn't work, cause I don't even thought that problem could be in your package. I have used your package in same manner as node's default spawn function and used types from node for it. Could you please, rethink your decision and accept a PR for it, or may be add mention of this behavior in readme with link to cross-spawn-with-kill?

@MohamedLamineAllal
Copy link

MohamedLamineAllal commented Apr 24, 2019

@satazor What's wrong with accepting the PR ? the function is pretty handy.

@satazor
Copy link
Contributor

satazor commented Apr 25, 2019

You may read the thread starting at this comment on why it didn’t make it: #44 (comment)

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

No branches or pull requests

5 participants