gdi3d / mute-spotify-ads-mac-osx

Mute Mac (osx) computer audio when Spotify plays an AD
https://gdi3d.github.io/mute-spotify-ads-mac-osx/
MIT License
120 stars 16 forks source link

Review some echos in shell script #41

Closed Alberdi closed 1 year ago

Alberdi commented 1 year ago

There was an extraneous "\r\n" that was always printed in the default terminal, so I removed that one and fixed some other minor typos here and there.

gdi3d commented 1 year ago

Hello!, thanks for the contribution. But can you share more info about the printed \r\n. What shell are you using and could you provide a screenshot?

Alberdi commented 1 year ago

Sure! I'm using the default Terminal (version 2.13) from macOS Ventura 13.4, with a zsh shell.

This is how I see it:

sc1

I did some investigations and found the following. From zsh:

ᐅ which echo
echo: shell built-in command
ᐅ echo "Test \r\n"  
Test 

ᐅ echo -e "Test \r\n"
Test 

From bash:

$ which echo
/bin/echo
$ echo "Test \r\n"
Test \r\n
$ echo -e "Test \r\n"
Test 

And, since the script invokes /bin/bash in the first line, I also get the \r\n displayed when running it. Changing it to /bin/sh makes it output the empty line in my setup, as it does changing the echo to echo -e. But I'm not sure what's a better solution; I could do either as a new commit in this PR if you so desire.

gdi3d commented 1 year ago

I believe a one-fit-all solution could be just removing the \r\n and adding an empty echo to generate the new lines.

If you can make that change and also update CURRENT_VER (line 2) to 23 (+1) I'll be glad to accept the PR