`limit` in the /message request should be a "smart" field about ruma HOT 6 CLOSED

… that is, instead of being Optional, it should have a default value when deserializing, like the limit field in get_context.

from ruma.

sync_events also has a struct with a field that should be updated in the same way.

from ruma.

Hi, I'm curious about the implementation.

Changing limit field type from Option<UInt> to UInt will make it required from the perspective of the library consumer. Is it intended to be that way? I'm asking this under the assumption that ruma-client-api will be used by developer of rust-based matrix client. CMIIW and go easy on me because I'm new.

Anyway here's a fix #66

from ruma.

Yes, this means that client authors will have to write UInt::from(0u32) where they could previously write None. This loss of convenience can be tackled by authors of higher-level client libraries (e.g. matrix-rust-sdk), although we're also planning to make construction of our types easier, by adding constructors to lots of our types; see #14.

from ruma.

I just thought that having a one-to-one mapping between Rust struct and its URI query component counterpart would reduce the programmer's cognitive load (in place of the struct being dumb)

Request { roomId: "some_room_id", from: "token", to: "token2", dir: Direction::backward, limit: None, filter: None  }
=> ?from=token&to=token2&dir=b

Request { roomId: "some_room_id", from: "token", to: "token2", dir: Direction::backward, limit: Some(10), filter: None  }
=> ?from=token&to=token2&dir=b&limit=0

I am assuming that the default: 10 part is already the server's responsibility. I haven't been able to find that it is actually the server's responsibility in the draft docs.

Also, having the client responsible for the default value, you might also want to expose the default value as public.

from ruma.

I am assuming that the default: 10 part is already the server's responsibility.

It is. But since ruma-client-api can also be used for servers, it makes sense to encode these kinds of defaults in our code, so people building servers on top of our libraries don't have to care about it.

Also, having the client responsible for the default value, you might also want to expose the default value as public.

We could do that, yeah. I'd not be opposed to that, if post #14 being fixed somebody still has a clear use case for it.

By the way, that issue is something that will have to be tackled as lots of independent small changes – if you want, you can open a PR adding a constructor for the Request you just modified (making it #[non_exhaustive] has to be done in macro code, we'll figure out how to best do that separately), with only the fields that the spec requires as arguments, the rest being set to their default values.

from ruma.