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

Fix incorrect process name cutting on macos #55

Merged
merged 6 commits into from
Dec 26, 2022

Conversation

jopemachine
Copy link
Contributor

@jopemachine jopemachine commented Nov 21, 2022

I found ps-list returns incorrect process name while working on sindresorhus/fkill-cli#83

I think it is broken in ps-list@7.0.0 (https://github.com/sindresorhus/ps-list/blob/v7.0.0/index.js#L125).

This PR will fix sindresorhus/fkill-cli#78.

Only tested in macOS yet.

Before

2022-11-21_19-07-06

After

afte


Fixes #49

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@jopemachine
Copy link
Contributor Author

jopemachine commented Nov 23, 2022

I'm so sorry, I found my patch was totally wrong.

Using replaceAll with incorrect regexp (should be with g flag) caused TypeError and the error callednonWindowsMultipleCalls, that's why it passed the tests.

The cutting is not caused by fkill's logic, it is by ps itself.

For example, I found ps wwxo comm,args command prints below.

...
/Applications/Vi /Applications/Visual Studio Code.app/Contents/Frameworks/Code Helper.app/Contents/MacOS/Code Helper --ms-enable-electron-run-as-node /Users/jopemachine/.vscode/extensions/samverschueren.linter-xo-3.14.1/dist/server.js --node-ipc --clientProcessId=17051
...

comm here is /Applications/Vi, which is already cut off.

I found this posting, https://unix.stackexchange.com/questions/401780/ps-cuts-command-how-to-get-full

This posting said to swap the argument order, but it didn't work since comm and args both are too long.

I think maybe the sole solution here is calling ps wwxo pid,args separately with ps wwxo pid,ppid,uid,%cpu,%mem,comm.

It increases the runtime cost, but still better than calling nonWindowsMultipleCalls, and it enables to return correct names.

If you agree, I will try to edit this patch in this direction.

@sindresorhus
Copy link
Owner

Yes, that is correct.

I should never have merged 285f434

@sindresorhus
Copy link
Owner

Have you tested these changes with fkill-cli?

@jopemachine
Copy link
Contributor Author

Have you tested these changes with fkill-cli?

Yes, and I also have tested below simple test code.

import psList from './index.js';
console.log(await psList({all: true}));

@sindresorhus sindresorhus merged commit 8acb5c4 into sindresorhus:main Dec 26, 2022
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.

Fails to show process name sometimes Process list not displaying application names
2 participants