Make `room_alias_or_id::Variant` a public enum about ruma HOT 10 OPEN

In most cases we use this when we receive a Matrix URI, or when the user wants to join a room by alias or ID, we need to differentiate between both:

• So we can find the room locally, we need to know if we should match the room ID or an alias,
• If we want to get the public room info, since there is no API for it currently in the spec we use the /hierarchy endpoint, but it works only with room IDs.

from ruma.

Technically, this still needs at least a PR to advertise the TryFrom APIs to be resolved.

from ruma.

Yeah, this is somewhat of a weird API. We used to have RoomOrAliasId::into_either as an optional thing (because it depended on the either crate), but I don't think anybody was using it. Speaking of, I don't actually know whether anybody uses this impl (or the one for RoomAliasId).

That said, this is not such an uncommon API. std also has APIs where the E of Result<T, E> isn't actually an error. I'm inclined to keep the API as-is.

@zecakeh do you have an opinion on this?

from ruma.

Yeah, this is somewhat of a weird API. We used to have RoomOrAliasId::into_either as an optional thing (because it depended on the either crate), but I don't think anybody was using it. Speaking of, I don't actually know whether anybody uses this impl (or the one for RoomAliasId).

That said, this is not such an uncommon API. std also has APIs where the E of Result<T, E> isn't actually an error. I'm inclined to keep the API as-is.

@zecakeh do you have an opinion on this?

Isn't the either crate a transitive dependency (is that the correct term?) Of Ruma anyways? Having a fresh look on this I guess I understand now, RoomAliasId is the error variant because it just exists for human convenience but can be turned into RoomId through the API.

from ruma.

I guess I understand now, RoomAliasId is the error variant because it just exists for human convenience

That is not the case. The "opposite" impl for &RoomAliasId where it is the Ok and &RoomId is the Err also exists:

impl<'a> TryFrom<&'a RoomOrAliasId> for &'a RoomAliasId {
    type Error = &'a RoomId;

    fn try_from(id: &'a RoomOrAliasId) -> Result<&'a RoomAliasId, &'a RoomId> {
        match id.variant() {
            Variant::RoomAliasId => Ok(RoomAliasId::from_borrowed(id.as_str())),
            Variant::RoomId => Err(RoomId::from_borrowed(id.as_str())),
        }
    }
}

So you can do both RoomAliasId::try_from(room_or_alias_id) and RoomId::try_from(room_or_alias_id).

from ruma.

As a client dev, I wasn't even aware that this was a possibility to get one type or the other and I am pretty we implemented something in Fractal to be able to match an enum for this.

So we need to document this API on the type's description in my opinion. I don't really prefer Either over Result. And I don't think this Variant enum is useful publicly. If we make an enum public it should probably use newtype variants to get the typed strings directly.

from ruma.

Would be interesting to know what Fractal uses this for!

from ruma.

So, should I close this issue now that the decision has been made?

from ruma.

Technically, this still needs at least a PR to advertise the TryFrom APIs to be resolved.

Could you elaborate on this?

from ruma.

We need to spell out in the docs of the types that the user can convert an (Owned)RoomOrAliasId to an (Owned)RoomId or (Owned)RoomAliasId by using the TryFrom/TryInto implementations.

from ruma.