kaustubhhiware / NotiFyre

Notify when a terminal task is done/ Terminal task notifier https://kaustubhhiware.github.io/NotiFyre/
MIT License
59 stars 13 forks source link

Use command when relying on external executables #1

Closed romainrichard closed 7 years ago

romainrichard commented 7 years ago

Whenever a command is called without the command prefix in Fish, one cannot guarantee what will get executed. See https://fishshell.com/docs/2.0/commands.html#command for details.

If for some reason, a user created a function named, for example, notify-send then calling notify-send without the command prefix will run the user's function. Best case scenario the user set the function to a non-disruptive behavior (adding some logging, sending an email, ... while still calling notify-send with the passed arguments). Worst case scenario the user set the function to do something totally unexpected (for example calling ls instead of notify-send.

The current code relies on notify-send (and a few others) to be their default built-in versions, the only way to ensure that is to add the command prefix.

kaustubhhiware commented 7 years ago

Thank you for the PR, @romainrichard :smile:

However, did you try running the notifier with your modifications ?

This is what I got when only a new terminal was started.

No command 'string' found, did you mean:
 Command 'strings' from package 'binutils-multiarch' (main)
 Command 'strings' from package 'binutils' (main)
 Command 'spring' from package 'ruby-spring' (universe)
 Command 'spring' from package 'spring' (universe)
string: command not found
~/.config/fish/functions/prompt_pwd.fish (line 20):         command string replace -ar '(\.?[^/]{'"$fish_prompt_pwd_dir_length"'})[^/]*/' '$1/' $tmp
                                                                    ^
in function “prompt_pwd”

You need to remove command before string in prompt_pwd.fish. Please make another commit for that.

kaustubhhiware commented 7 years ago

Seems good :+1: