franciscolourenco / done

A fish-shell package to automatically receive notifications when long processes finish.
MIT License
767 stars 70 forks source link

Don't fail when powershell.exe is missing #67

Closed 9999years closed 4 years ago

9999years commented 4 years ago

On some platforms, uname -a contains Microsoft but powershell.exe is not in $PATH, so if powershell.exe ... 2>/dev/null still displays an error:

if powershell.exe -command "Import-Module -Name BurntToast" 2>/dev/null; echo true; end
powershell.exe: command not found
fish:
if powershell.exe -command "Import-Module -Name BurntToast" 2>/dev/null; echo true; end
   ^

Add a check to command -v powershell.exe to avoid the error message.

9999years commented 4 years ago

Here's the error that I get e.g. after running fisher:

powershell.exe: command not found
~/.config/fisher_local/conf.d/done.fish (line 158):
                if powershell.exe -command "Import-Module -Name BurntToast" 2>/dev/null
                   ^
in function '__done_ended'
        called on standard input

in event handler: handler for generic event 'fish_prompt'
franciscolourenco commented 4 years ago

@9999years what platform are you running this in? Would the default bell sound work in your case?

echo -e "\a" # bell sound
9999years commented 4 years ago

The bell sound works!

This happens on WSL 1 on my laptop, but not on my desktop (strange, because they're theoretically identical systems).

We could also get the PowerShell path on WSL with something like:

set -l powershell_exe (wslpath (wslvar windir)/System32/WindowsPowerShell/v1.0/powershell.exe)
test -x "$powershell_exe"

and then:

> env "$powershell_exe" -command "Import-Module -Name BurntToast"
Import-Module : The specified module 'BurntToast' was not loaded because no valid module file was found in any module
directory.
At line:1 char:1
+ Import-Module -Name BurntToast
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ResourceUnavailable: (BurntToast:String) [Import-Module], FileNotFoundException
    + FullyQualifiedErrorId : Modules_ModuleNotFound,Microsoft.PowerShell.Commands.ImportModuleCommand
franciscolourenco commented 4 years ago

Sounds like we should do both. Improve the chances of getting the powershell binary, and not fail if we don't. I think we should also join the if condition at the highest level, so that we can proceed to play the bell sound in case the one of the requirements fails.

@9999years would you mind making these changes? Thanks!

9999years commented 4 years ago

Made the changes and made sure the logic works on WSL with BurntToast both uninstalled and installed.

franciscolourenco commented 4 years ago

Looks good, thanks!