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

defmt: implements Format for cells #656

Merged
merged 1 commit into from
Feb 15, 2022
Merged

defmt: implements Format for cells #656

merged 1 commit into from
Feb 15, 2022

Conversation

DBLouis
Copy link
Contributor

@DBLouis DBLouis commented Jan 29, 2022

This patch implement the Format trait for Cell<T> and RefCell<T>

@Urhengulas
Copy link
Member

Hi @DBLouis, Thank you for your PR! Could you please move your code to defmt/src/impls/core_/cell.rs and add that as a new mod?

@Urhengulas
Copy link
Member

Thank you!

Secondly, I'd propose to stay close to the representations in std.

This code:

use std::cell::{Cell, RefCell};

fn main() {
    let a = Cell::new(5);
    println!("{:?}", a);

    let b = RefCell::new(5);
    println!("{:?}", &b);

    let _c = b.try_borrow_mut().unwrap();
    println!("{:?}", &b);

    let d = b.try_borrow_mut().unwrap_err();
    println!("{:?}", d);
}

Produces that output:

Cell { value: 5 }
RefCell { value: 5 }
RefCell { value: <borrowed> }
BorrowMutError

(I couldn't figure out how to trigger a BorrowError, but it should be just "BorrowError")

Do you want to quickly change that? Else I can also do it.

@DBLouis
Copy link
Contributor Author

DBLouis commented Feb 12, 2022

You can trigger a BorrowError by borrowing mutably first then attempting to borrow immutably Playground

@Urhengulas
Copy link
Member

Thank you!

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 15, 2022

Build succeeded:

@bors bors bot merged commit 9bbb0a9 into knurling-rs:main Feb 15, 2022
bors bot added a commit that referenced this pull request Feb 15, 2022
622: Refactor rtt [2/2] r=Urhengulas a=Urhengulas

This PR contains the not-error-prone changes from #573 and addresses the review given to #573.

661: `snapshot-tests`: Add tests for cell types r=Urhengulas a=Urhengulas

Follow-up to #656, which add snapshot tests for the new `defmt::Format` implementations.

Co-authored-by: Johann Hemmann <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants