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 issue #58 and issue #42: crashes on Windows 11 24H2 and wrong results on processes with spaces in name #62

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BePo65
Copy link

@BePo65 BePo65 commented Nov 26, 2024

Solves issue #58 : On windows systems ps-tree uses WMIC to get the list of child processes. WMIC is deprecated since 2016. This PR replaces WMIC with powershell (as recommended by Microsoft),
I encountered this problem, when using the start-server-and-run package (see bahmutov/start-server-and-test#384).

Solves issue #42 - ps-treee returns wrong results when process name contains spaces (primarily on windows systems). Issue is caused by splitting a row into columns on spaces. Fixed by getting the column widths, when extracting the headers (all columns have fixed width).

index.js Outdated Show resolved Hide resolved
Co-authored-by: Mike McCready <[email protected]>
@nelsonic
Copy link
Collaborator

Running build/tests on GitHub CI just so progress can be made on this PR ... 🤞

@colin969
Copy link

colin969 commented Dec 22, 2024

Any chance this can get looked at? I'm starting to see failures on new regular Windows installs from ps-tree now that wmic isn't included by default, so it's going to start affecting significantly more people soon.

Copy link
Collaborator

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code updates LGTM. 👌
Could you please add a windows-specific test that will run on GitHub Actions CI to confirm this us working and avoid future regressions when something changes.

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Dec 22, 2024

@nelsonic

Could you please add a windows-specific test that will run on GitHub Actions CI to confirm this is working and avoid future regressions when something changes.

https://github.com/indexzero/ps-tree/pull/62/checks already shows successful tests on Windows

The branch BePo65:pu/issue58 needs to be rebased on the master branch, or the master branch needs to be merged into the branch for this PR.

Before any more work is done on this repo there needs to be confirmation from the owner @indexzero that there will be a possibility to release a new version, otherwise there is no real benefit from any fixes.

I checked that this branch runs successfully on windows-2025. That should be added to the list of tested operating systems in https://github.com/indexzero/ps-tree/blob/master/.github/workflows/ci-test.yml

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.

4 participants