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

Use one ps execution instead of multiple when possible #30

Merged
merged 11 commits into from Jun 20, 2019
Merged

Use one ps execution instead of multiple when possible #30

merged 11 commits into from Jun 20, 2019

Conversation

stroncium
Copy link
Contributor

@stroncium stroncium commented Mar 17, 2019

Ok, so I examined all ps implementations I found and it turns out they have one thing in common: they print comm and args values(doesn't apply to headers) left aligned.

Using this and the fact we run ps itself with controlled arguments, we parse ps output, find the line with ps itself, find out locations of comm and args and length of comm and use this info to parse all the lines. Afterwards, we exclude ps itself from resulting process list(which was a side-effect of previous implementation and probably is the right thing to do anyway).

Fixes #14

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Can you give the PR a proper title?

@stroncium stroncium changed the title fix #14 Use one ps execution instead of multiple when possible, fixes #14 Jun 14, 2019
@sindresorhus sindresorhus changed the title Use one ps execution instead of multiple when possible, fixes #14 Use one ps execution instead of multiple when possible Jun 15, 2019
index.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Would be good to maybe add some more tests.

@stroncium
Copy link
Contributor Author

@sindresorhus It's quite hard to test it for good, but I've added one more test where most parameters(except mem and cpu) are controlled.

@sindresorhus sindresorhus merged commit 285f434 into sindresorhus:master Jun 20, 2019
@medusalix medusalix mentioned this pull request Mar 15, 2020
@sindresorhus
Copy link
Owner

@stroncium I upgraded ps-list to this version in fkill-cli and it changed from showing the process name to showing the full path truncated...

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

Successfully merging this pull request may close these issues.

Converting from four command executions to one
2 participants