Windows compatible (fix)

Maxim-Mazurok commented 2 years ago

https://github.com/jaredly/vscode-background-terminal-notifier/pull/8 didn't work on my PC (Windows 11) - ps-node was timing out even when I was running it by itself (maybe related to https://github.com/neekey/ps/issues/75)

I've tried process-list but it was giving me hard time with native prebuilds.

ps-list worked like a charm, but it doesn't provide command on Windows, which is fine by me.

Also notifications didn't work when command/message is undefined or an empty string, seems like it needs to have at least some content.

Also sounds didn't work at first, so I used speaker package, but then it turned out that I just disabled all notification sounds in system:

To build/install:

git clone https://github.com/Maxim-Mazurok/vscode-background-terminal-notifier
git checkout windows-compatible
npm i vscode@latest
npm ci
npm i -g vsce
vsce package
code --install-extension .\background-terminal-notifier-1.0.5.vsix
reload

Sometimes vsce package hangs, just Ctrl+C and proceed, it actually worked fine.

If code --install-extension .\background-terminal-notifier-1.0.5.vsix hangs - close VS Code and delete c:\Users\{YourUserName}\.vscode\extensions\jaredly.background-terminal-notifier-1.0.5 manually

jaredly commented 1 month ago

So currently this PR would be a regression on non-windows systems; could you make it so it only uses ps-list on windows?

Maxim-Mazurok commented 1 month ago

I just wanted to share the fix that worked for me, but sure, I could look into polishing it up, and have access to all other OSes to test. Not sure why this one would be a regression tho, I see in ps-list readme this:

Works on macOS, Linux, and Windows.

It's been a few years, but at first glance it looks like it was supposed to work fine on other platforms as well. In any case, let me know if you're actually interested in getting this finalized and what issues you had with this, and I'll have a closer look, cheers!

jaredly commented 1 month ago

The regression is that ps-list doesn't support showing the original command, according to the original description.

ps-list worked like a charm, but it doesn't provide command on Windows, which is fine by me.

Maxim-Mazurok commented 1 month ago

Mm, so IIRC the command will continue to be shown on MacOS/Linux as before, so not a regression. And Windows never worked, so adding its support is also not a regression. I mean if ps-list can't do this - I won't be trying to fix ps-list itself... My hunch is that it might not be possible on Windows, like even in Task Manager you'd just see cmd.exe or something, so not sure if it's possible to get a command. From the code it seems like it should work on other OSes as before tho, so I woudn't call this a regression :) Hope this makes sense!

jaredly commented 1 month ago

Ah, I didn't realize that is what you meant; in that case I'll test it out to verify it still works as before. Thanks!

On Sat, Oct 5, 2024, 11:27 PM Maxim Mazurok @.***> wrote:

Mm, so IIRC the command will continue to be shown on MacOS/Linux as before, so not a regression. And Windows never worked, so adding its support is also not a regression. I mean if ps-list can't do this - I won't be trying to fix ps-list itself... My hunch is that it might not be possible on Windows, like even in Task Manager you'd just see cmd.exe or something, so not sure if it's possible to get a command. From the code it seems like it should work on other OSes as before tho, so I woudn't call this a regression :) Hope this makes sense!

— Reply to this email directly, view it on GitHub https://github.com/jaredly/vscode-background-terminal-notifier/pull/9#issuecomment-2395288943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA3MKS5X5R3UDUT7BZKHWTZ2C32ZAVCNFSM6AAAAABPLRV76OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGI4DQOJUGM . You are receiving this because you commented.Message ID: @.*** com>

jaredly / vscode-background-terminal-notifier

Windows compatible (fix) #9