EdJoPaTo / website-stalker

Track changes on websites via git
GNU Lesser General Public License v2.1
54 stars 6 forks source link

chore(run): Print error without panic when config is invalid. #177

Closed Teufelchen1 closed 1 year ago

Teufelchen1 commented 1 year ago

Hi! 🦓

The Rust docs say about expect(): Because this function may panic, its use is generally discouraged. Instead, prefer to use pattern matching and handle the [Err](https://doc.rust-lang.org/std/result/enum.Result.html#variant.Err) case explicitly[..]

I tried to fix this accordingly.

EdJoPaTo commented 1 year ago

Because this function may panic

expect is mostly discouraged for library code and situation where it could be possible to handle the error gracefully. In this case the process is stopping early either way.

My personal way of approaching them (I'm not perfect there either): unwrap only in tests or Lazy, expect when there is no way to continue the process and Result when something could continue. Library code shouldn't use panics at all.

In this case the process is ending either way. Only the output is a bit nicer. At the cost of more code (in a function with clippy::too_many_lines already allowed 😬)

EdJoPaTo commented 1 year ago

Still not sure what to think about this. I both like cleaner output in error cases and I like the shorter code of the expect function…

Teufelchen1 commented 1 year ago

While working on this, I experimented with building this error message & clean exit into the config loader. This ofc slightly altered the behavior / output of the check command. Would you like that more? I personally rejected that approach as I found it more clear to read, when the error is propagated up, instead of a having a config loader which "randomly" exists the executable.

EdJoPaTo commented 1 year ago

I think I prefer the .expect currently. Its a one liner which is exactly doing what is needed here: Printing out a message when its an Err case.

In the long run thinking about https://github.com/EdJoPaTo/website-stalker/discussions/172 is probably the best way to improve usability.

Thank you for thinking about ways to improve the tool. Rethinking some ways is always valuable!