elliot40404 / bonk

The blazingly fast touch alternative with a sprinkle of mkdir written in rust.
MIT License
97 stars 9 forks source link

Errors will panic #5

Closed BartMassey closed 1 year ago

BartMassey commented 1 year ago

Instead of panicing on error, bonk should cleanly exit with status 1.

elliot40404 commented 1 year ago

@BartMassey Can you please show me a situation where bonk panics? Thanks

pacak commented 1 year ago

Try making a file with non-utf8 name

elliot40404 commented 1 year ago

@pacak Can you please refer me to an example/link or code snippet where the program exits cleanly with a suitable error code on panic ?

pacak commented 1 year ago

If you want a clean exit - you don't panic and use Result with or without custom errors.

BartMassey commented 1 year ago

You can use std::process::exit as long as you are sure that you don't have things that need to be cleaned up by unwinding. Otherwise, you can use std::panic::resume_unwind for an error exit as long as you don't have other threads running.

The problem with using an error return from main is that the formatting is not going to be too user-friendly.

pacak commented 1 year ago

The problem with using an error return from main is that the formatting is not going to be too user-friendly.

Pattern match around that area, print a user friendly error and use std::process::exit.

elliot40404 commented 1 year ago

@pacak @BartMassey Would you please try out the new release [v0.3.2] before I close this issue. Thanks

pacak commented 1 year ago
elakelaiset% ls -lha
total 56K
drwxrwxr-x 2 pacak pacak 4.0K Oct  6 14:15 .
drwxrwxrwx 3 pacak pacak  52K Oct  6 14:15 ..
elakelaiset% touch  "hello \xe0 world\\ xxxx"
elakelaiset% ls -lha
total 56K
drwxrwxr-x 2 pacak pacak 4.0K Oct  6 14:15  .
drwxrwxrwx 3 pacak pacak  52K Oct  6 14:15  ..
-rw-rw-r-- 1 pacak pacak    0 Oct  6 14:15 'hello \xe0 world\ xxxx'
elakelaiset% rm hello\ \\xe0\ world\\\ xxxx
elakelaiset% ls -lha
total 56K
drwxrwxr-x 2 pacak pacak 4.0K Oct  6 14:15 .
drwxrwxrwx 3 pacak pacak  52K Oct  6 14:15 ..

Try the same with bonk.

As for the error handling - there's a bunch of code duplication. I'd use ? and handle them from one place. Still a bunch of unwraps.

pacak commented 1 year ago

You can print _e to be a bit more concrete but that's better, yes.

BartMassey commented 1 year ago

All those unwraps are still going to result in panics. Install http://github.com/BartMassey/fromhex and then try

$ bonk /tmp/test`fromhex e0`

You really need to convert the OsStrings returned from args_os() into Paths or PathBufs ASAP, and only work with PathBufs and Paths after that. Otherwise dealing with non-UTF8 paths is going to be hopeless.

pacak commented 1 year ago

https://github.com/BartMassey/fromhex

that's what \xe0 did in my example. It also touches some other issues.

BartMassey commented 1 year ago

that's what \xe0 did in my example.

Didn't work for me: different shell maybe, or something.

$ echo '\xe0' >/tmp/test
$ hd /tmp/test
00000000  5c 78 65 30 0a                                    |\xe0.|
00000005

Wanted to provide an executable that would reliably produce an illegal value.

elliot40404 commented 1 year ago

@BartMassey What would say would be the best way to deal with rest of the unwraps and how do I perform the string operations I am currently doing on the PathBufs

BartMassey commented 1 year ago

Instead of unwrapping, you should be able to use ? for most of those given your current structure. The places where you are unwrapping an Option instead of a Result can be handled with ok_or()? and ok_or_else()?

For the separate issue of not using UTF-8 str types as paths I'd take a look at http://github.com/BartMassey/touche, which contains a decent example to follow you can look at what methods are provided on the Path and PathBuf types. You won't be able to do arbitrary string manipulation, but should be able to do everything you need for the job with the provided methods.