Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature req: defmt::expect! macro like unwrap! #867

Open
dvdsk opened this issue Sep 27, 2024 · 5 comments
Open

feature req: defmt::expect! macro like unwrap! #867

dvdsk opened this issue Sep 27, 2024 · 5 comments

Comments

@dvdsk
Copy link

dvdsk commented Sep 27, 2024

The unwrap! macro got implemented see , in that issue a expect! macro is also proposed and an implementation sketched out . Only the unwrap! macro got implemented however. Is there a reason for this?

To me an expect! macro would be very useful .

@Urhengulas
Copy link
Member

It would be useful indeed. Are you interested in implementing it?

@dvdsk
Copy link
Author

dvdsk commented Sep 30, 2024

I'll have a go at it, not familiar with the internals of defmt however since there is a copypaste-ready implementation and I can have a look at unwrap! I can probably figure it out. Then we can hammer out the details in the PR 👍

@dvdsk
Copy link
Author

dvdsk commented Oct 1, 2024

So I had a quick look at noticed that unwrap! already implements the behavior of expect!. As in it allows a second argument that is a message. So expect! is implemented as unwrap!. See the docs here: https://docs.rs/defmt/latest/defmt/macro.unwrap.html

So this seems to be more an issue of discoverability then missing features. What to do there. I propose we make expect an alias for the current unwrap-macro. That way users that know of unwrap! can discover expect!. It might also be worth mentioning unwrap! in the defmt book. If I am not mistaken its not in there atm.

@Urhengulas
Copy link
Member

So this seems to be more an issue of discoverability then missing features. What to do there. I propose we make expect an alias for the current unwrap-macro. That way users that know of unwrap! can discover expect!.

Agree

It might also be worth mentioning unwrap! in the defmt book. If I am not mistaken its not in there atm.

and agree

dvdsk added a commit to dvdsk/defmt that referenced this issue Oct 9, 2024
See related issue: knurling-rs#867
Its easy to assume unwrap! can not have a message like std's expect.

Adding an expect macro will help both autocomplete guide programmers
looking for one and those who discount the unwrap entry in the defmt
api docs based on their knowledge that std's unwrap can not have a
message.

The documentation of the expect alias highlights that unwrap works with
a message to and points to the unwrap macro docs for details.
@dvdsk
Copy link
Author

dvdsk commented Oct 9, 2024

PR #872 for the alias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants