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

Bump matrix-sdk dependency to 0.10.0 #397

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kenrachynski
Copy link

There is a warning that popped up during this change and I have no idea how to deal with it.

warning: elided lifetime has a name
   --> src/message/mod.rs:964:29
    |
957 |     pub fn show_with_preview<'a>(
    |                              -- lifetime `'a` declared here
...
964 |     ) -> (Text<'a>, Option<(&dyn Protocol, u16, u16)>) {
    |                             ^ this elided lifetime gets resolved as `'a`
    |
    = note: `#[warn(elided_named_lifetimes)]` on by default

Otherwise, this seems to be running fine

@kenrachynski
Copy link
Author

Comment from @bnhbvr in #iamb-dev:

i think that history_visibility() became something returning an Option now, so using {:?} when rendering it in a formatted string will show None or Some(…), which is not super nice for the user. Might prefer something like room.history_visibility().map(ToString::to_string).unwrap_or_else(|| "<unknown>".to_owned())⁦
hmm that might fit in a single map_or_else call, actually  

@kenrachynski kenrachynski force-pushed the upgrade-matrix-sdk-0.10.0 branch from b1244a0 to e2fd5d1 Compare February 24, 2025 17:44
@@ -572,7 +572,7 @@ impl RoomState {
let msg = match field {
RoomField::History => {
let visibility = room.history_visibility();
format!("Room history visibility: {visibility}")
format!("Room history visibility: {visibility:?}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reiterating my comment in the right place :)

This is where we're calling Room::history_visibility(), which will now return an Optional value, None meaning the value is unknown because the related state event hasn't been sync'd yet.

I think it might be nicer in terms of UX to display it as "unknown" when it's set to None, instead of rendering None (and then we can also keep the current formatting when it's known, instead of Some(Public) or somesuch).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I'm not sure I even like the Some("shared") when I do :room history show. Good to know where it displays, at least.

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