rust-si icon indicating copy to clipboard operation
rust-si copied to clipboard

Add `@impl or_none` to try_scan! (#28)

Open kangalio opened this issue 4 years ago • 11 comments

This is my attempt at implementing #28

kangalio avatar Jul 24 '20 09:07 kangalio

Cool! Please also add a test to our test suite showing a use of it

oli-obk avatar Jul 24 '20 11:07 oli-obk

I added a test, and also documented this new feature.

On another note, the following two macro variants seem to be more of an implementation detail than a part of the public API

(@question_mark: $($e:tt)+) => { ... };
(@unwrap: $($e:tt)+) => { ... };

Do you agree that these two macro variants should be removed from the docs?

kangalio avatar Jul 24 '20 21:07 kangalio

All of the @foo patterns are meant to be internal. You're not supposed to invoke them directly, but there should be convenient wrappers for them.

oli-obk avatar Jul 25 '20 14:07 oli-obk

For example: the scan! macro invokes the @impl unwrap variant. So I don't think we should be exposing these variants in the documentation. We can add a scan_opt! macro that invokes try_scan with @impl or_none

oli-obk avatar Jul 25 '20 14:07 oli-obk

It is a very convenient feature to be able to choose how errors will be handled. I'd personally pledge for exposing the @impl ... feature. Maybe it can be renamed to just @... or something else entirely, in order to make it more intuitive and approachable to users.

If you disagree, I suppose a wrapper macro is also a viable option.

kangalio avatar Jul 25 '20 16:07 kangalio

hmm... I like that. So we get rid of the impl and just use @mode for choosing one of the modes.

oli-obk avatar Jul 25 '20 16:07 oli-obk

We should probably rename the modes to something more user friendly then. What do you think about the following list?

  • @unwrap or @panic
  • @return_err or @?
  • @option

oli-obk avatar Jul 25 '20 16:07 oli-obk

at that point we could also start supporting these flags directly on scan! instead of try_scan!

oli-obk avatar Jul 25 '20 16:07 oli-obk

@unwrap or @panic @return_err or @? @option

Those look like good ideas. Let me add my own:

@unwrap or @panic @return_err or @? @return_none or @option or @or_none

If I had to choose one each, hmm... I think I would choose @panic + @return_err + @return_none. Those are very clear names because they describe exactly what is gonna happen on failures.

kangalio avatar Jul 25 '20 16:07 kangalio

I'm not sold on the capitalization of panic as Panic, but other than that, I agree with your selection, those keywords make it very clear.

oli-obk avatar Jul 25 '20 17:07 oli-obk

Oh I'm only seeing now that GitHub capitalized my panic. It was supposed to be lowercase

kangalio avatar Jul 25 '20 17:07 kangalio