search

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

Clean up code, introduce easier configuration #6

Closed icyflame closed 7 years ago

icyflame commented 7 years ago

The earlier code didn't use if and tried to do everything with ||. Using if-then-else makes the whole script a lot more readable. Is there any particular reason (eg: performance) that you used that instead of if-then-else?

  1. I have moved all of the configuration to the top of the file.
  2. Cut down on the code to have the main block of code only once, instead of having it twice as before
  3. Use if-else instead of ||
  4. Show the exit status of the last command in the notification

If you want debug logs, you can check out the script at 06faa4b6d704655c529a5741c2e1753240325c01.

I don't understand what the $IFS part does. Can you please explain? I am using zsh and there's no IFS variable in my env.

kaustubhhiware commented 7 years ago

Thanks for your contribution @icyflame ! If it's ready from your side, I'll merge it.

icyflame commented 7 years ago

@kaustubhhiware Okay, great. I need to update the readme. So, I will do that and ping you here once again. You can review and merge then! Thanks! :slightly_smiling_face:

icyflame commented 7 years ago

@kaustubhhiware I am done with the readme now. It was a little shabby (markdown is weird about each list item's text, it left aligns it instead of indenting properly)

I have written the configuration section for zsh and bash as well. I hope you can understand it. If not, let me know what you have trouble getting, I will fix it.

kaustubhhiware commented 7 years ago

Looks perfect ! Could you squash some commits (say reduce it to 2-4) ? This project isn't that big, that is the only reason I'm asking this.

icyflame commented 7 years ago

@kaustubhhiware done :smile: