From 05a41d3b4dd0d2170a3dd0aac27cd20e10c05ab9 Mon Sep 17 00:00:00 2001 From: Denis Kasak Date: Fri, 10 Jul 2020 15:47:11 +0200 Subject: [PATCH] Move and rename member_display_name to RoomMember::disambiguated_name. This makes more sense as all the required information is now available on `RoomMember`. We also don't have to handle the case of the missing member since now you have to actually get a `RoomMember` before you can ask for their name. --- matrix_sdk_base/src/models/room.rs | 51 +++++------------------ matrix_sdk_base/src/models/room_member.rs | 32 ++++++++++++-- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index d6e9c666..d2adbe79 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -13,7 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::TryFrom; @@ -319,34 +318,6 @@ impl Room { self.encrypted.as_ref() } - /// Get the disambiguated display name for a member of this room. - /// - /// If a member has no display name set, returns the MXID as a fallback. - /// - /// This method never fails, even if user with the supplied MXID is not a member in the room. - /// In this case we still return the supplied MXID as a fallback. - /// - /// When displaying a room member's display name, clients *must* use this method to obtain the - /// name instead of displaying the `RoomMember::display_name` directly. This is because - /// multiple members can share the same display name in which case the display name has to be - /// disambiguated. - pub fn member_display_name<'a>(&'a self, id: &'a UserId) -> Cow<'a, str> { - let member = self.get_member(id); - - match member { - Some(member) => { - if member.display_name_ambiguous { - member.unique_name().into() - } else { - member.name().into() - } - } - - // Even if there is no such member, we return the MXID that was given to us. - None => id.as_ref().into(), - } - } - /// Process the join or invite event for a new room member. /// /// This method should be called only on events which add new members, not on existing ones. @@ -1347,7 +1318,7 @@ mod test { { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); assert_eq!("example", display_name1); } @@ -1366,9 +1337,9 @@ mod test { { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); - let display_name2 = room.member_display_name(&user_id2); - let display_name3 = room.member_display_name(&user_id3); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); + let display_name3 = room.get_member(&user_id3).unwrap().disambiguated_name(); assert_eq!(format!("example ({})", user_id1), display_name1); assert_eq!(format!("example ({})", user_id2), display_name2); @@ -1385,7 +1356,7 @@ mod test { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); assert_eq!("example", display_name1); } @@ -1401,8 +1372,8 @@ mod test { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); - let display_name2 = room.member_display_name(&user_id2); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); assert_eq!(format!("example ({})", user_id1), display_name1); assert_eq!(format!("example ({})", user_id2), display_name2); @@ -1419,8 +1390,8 @@ mod test { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); - let display_name2 = room.member_display_name(&user_id2); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); assert_eq!("changed", display_name1); assert_eq!("example", display_name2); @@ -1437,8 +1408,8 @@ mod test { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); - let display_name2 = room.member_display_name(&user_id2); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); assert_eq!(format!("changed ({})", user_id1), display_name1); assert_eq!(format!("changed ({})", user_id2), display_name2); diff --git a/matrix_sdk_base/src/models/room_member.rs b/matrix_sdk_base/src/models/room_member.rs index f508a2b1..1be90dfb 100644 --- a/matrix_sdk_base/src/models/room_member.rs +++ b/matrix_sdk_base/src/models/room_member.rs @@ -105,7 +105,8 @@ impl RoomMember { } } - /// Returns the most ergonomic name available for the member. + /// Returns the most ergonomic (but potentially ambiguous/non-unique) name available for the + /// member. /// /// This is the member's display name if it is set, otherwise their MXID. pub fn name(&self) -> String { @@ -114,16 +115,39 @@ impl RoomMember { .unwrap_or_else(|| format!("{}", self.user_id)) } - /// Returns a name for the member which is guaranteed to be unique. + /// Returns a name for the member which is guaranteed to be unique, but not necessarily the + /// most ergonomic. /// - /// This is either of the format "DISPLAY_NAME (MXID)" if the display name is set for the - /// member, or simply "MXID" if not. + /// This is either a name in the format "DISPLAY_NAME (MXID)" if the member's display name is + /// set, or simply "MXID" if not. pub fn unique_name(&self) -> String { self.display_name .clone() .map(|d| format!("{} ({})", d, self.user_id)) .unwrap_or_else(|| format!("{}", self.user_id)) } + + /// Get the disambiguated display name for the member which is as ergonomic as possible while + /// still guaranteeing it is unique. + /// + /// If the member's display name is currently ambiguous (i.e. shared by other room members), + /// this method will return the same result as `RoomMember::unique_name`. Otherwise, this + /// method will return the same result as `RoomMember::name`. + /// + /// This is usually the name you want when showing room messages from the member or when + /// showing the member in the member list. + /// + /// **Warning**: When displaying a room member's display name, clients *must* use + /// a disambiguated name, so they *must not* use `RoomMember::display_name` directly. Clients + /// *should* use this method to obtain the name, but an acceptable alternative is to use + /// `RoomMember::unique_name` in certain situations. + pub fn disambiguated_name(&self) -> String { + if self.display_name_ambiguous { + self.unique_name().into() + } else { + self.name().into() + } + } } #[cfg(test)]