What about adding a param controlling if enable http2_only or not? So that we can just write such codes(example codes from Line 1068-1075):

                    auto::Builder::new(TokioExecutor::new())
                        .http1_only(h1_only)
                        .http2_only(h2_only)
                        .serve_connection(stream, service_fn(hello))
                        .await;

instead of

                    let mut builder = auto::Builder::new(TokioExecutor::new());
                    if h1_only {
                        builder = builder.http1_only();
                    } else if h2_only {
                        builder = builder.http2_only();
                    }

                    builder.serve_connection(stream, service_fn(hello)).await;

As far as I'm concerned, the latter snippet lacks elegance.

You're right in terms of ergonomics, but the first example somewhat suggests that we expect the user to turn the knob for both of them.

With the second example, while not elegant, does suggest that you set only either at any given time.

You're right in terms of ergonomics, but the first example somewhat suggests that we expect the user to turn the knob for both of them.

With the second example, while not elegant, does suggest that you set only either at any given time.

I must admit that I agree with you about avoiding the user's mistake of setting http1_only or http2_only multiple times. After all, there are always users who don't read the documentation carefully, or mistakenly do so, whick may bring with some unintended troubles.

However, considering that hyper-util is a relatively low-level library, should we provide more flexibility to users and let themselves be responsible for not setting http1_only or http2_only multiple times? BTW, in my opinion, manual runtime panic in release mode doesn't seem quite appropriate, since it's actually not an unrecoverable error, consider using debug_assert?

As @tisonkun pointed out, seems like a pretty straight forward case for exposing the enum. Why introduce the possibility of an error or require conditional logic.

async fn start_server(version: Option<Version>) -> SocketAddr {
    ...
    let mut builder = auto::Builder::new(TokioExecutor::new());
    builder = match version {
        Some(version) => builder.http_version(version),
        None => builder
    };
    builder.serve_connection(stream, service_fn(hello)).await;

I don't think http_version should accept an Option though, since None is already the default. But if you do allow, then

async fn start_server(version: Option<Version>) -> SocketAddr {
    ...
     auto::Builder::new(TokioExecutor::new())
            .http_version(version)
            .serve_connection(stream, service_fn(hello))
            .await;

I'd instead suggest:

    pub fn http_version(mut self, v: Option<Version>) -> Self

... and expose Version struct. Otherwise, it's panicable when you call with http1_only = true and http2_only = true, or self.http1_only().http2_only(). It's hard to tell if a passed builder is already configured. version setter override the configured value, at least better than assert! not set.