search

evanyeung / terminal-slack

Terminal client for slack
MIT License
1.03k stars 78 forks source link

Add private groups to channels view and throw fatal errors #35

Open omnibrian opened 7 years ago

omnibrian commented 7 years ago

Hey,

Since most of my use of slack is in private channels, I decided to add a feature for it. I've implemented it by changing the slackClient so I could keep the amount of files changed to a minimum. I also realise that checking for an error to see if it's a group isn't the best of solutions but it's the only one I could think of.

I've also added a second commit where I changed the error logging from logging to a file to throwing the error message. If you don't agree with that I can change to PR to just be the first commit.

Tell me what you think.

EDIT: After noticing that the slack api doesn't react well with multi-user direct message channel names I removed them with the same filter for removing archived channels.

Thanks, Brian

jaythomas commented 7 years ago

Did you intend to use the Error constructor?

throw new Error('SLACK_TOKEN undefined. Please add SLACK_TOKEN to the environment variables.')
omnibrian commented 7 years ago

Yes, I can change that to more appropriate error type if there is one, I just figured that throwing an error is cleaner than console logging an error message and exiting with a failed status.

jaythomas commented 7 years ago

I think throwing an error is best practice, I just meant that you are throwing a string instead of an Error object in your code the way you have it written now. It is generally best practice to wrap the string in new Error("my string") so that try/catch blocks can more easily handle the errors.

omnibrian commented 7 years ago

Ah, my bad, I'm still learning js. I've updated the PR to use the Error constructor as you suggested, thanks!

loflina commented 6 years ago

Thank you for this amazing tool! I love it! I'm sorry if this isn't the appropriate place for this but, can I ask why we don't show multi-user direct messages? This would be really helpful for me.