Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Oct 20 15:55
    nyarly commented #472
  • Oct 20 15:30
    msrd0 commented #483
  • Oct 19 16:13
    nyarly labeled #483
  • Oct 19 16:13
    nyarly opened #483
  • Oct 19 08:00
    dependabot-preview[bot] labeled #482
  • Oct 19 08:00
    dependabot-preview[bot] opened #482
  • Oct 19 08:00

    dependabot-preview[bot] on cargo

    Update tokio-rustls requirement… (compare)

  • Oct 16 07:24
    dependabot-preview[bot] labeled #481
  • Oct 16 07:24
    dependabot-preview[bot] opened #481
  • Oct 16 07:24

    dependabot-preview[bot] on cargo

    Update tokio requirement from 0… (compare)

  • Oct 14 14:41

    msrd0 on cargo

    (compare)

  • Oct 14 14:41

    msrd0 on master

    Update pin-project requirement … (compare)

  • Oct 14 14:41
    msrd0 closed #480
  • Oct 14 07:54
    codecov[bot] commented #480
  • Oct 14 07:34
    codecov[bot] commented #480
  • Oct 14 07:14
    dependabot-preview[bot] labeled #480
  • Oct 14 07:14
    dependabot-preview[bot] opened #480
  • Oct 14 07:14

    dependabot-preview[bot] on cargo

    Update pin-project requirement … (compare)

  • Oct 13 12:46

    msrd0 on cargo

    (compare)

  • Oct 13 12:46

    msrd0 on master

    Update nix requirement from 0.1… (compare)

Judson Lester
@nyarly
And I can report that the docs there could be improved. HandlerMarkeris an opaque type, and I think the explanation of the tradeoff there could back up a step. I'll put in a docs PR shortly.
Denis Lisov
@tanriol
@nyarly I need to think about this, but I think it would not. The problem is that we cannot write the bound as for<'s> Fn(&'s mut State) -> impl Future<...> + 's because impl Future cannot be used in that position.
Judson Lester
@nyarly
I'm also thinking, as I work with this, that it would be ideal not to have both methods :/. Maybe a future version has async_to and we deprecate to_async*
Denis Lisov
@tanriol
It could be just a .to(...) that works with everything... if it worked.
Judson Lester
@nyarly
I think we'd have async fn wrapper(&'s mut State) -> Resultand then take the handler and pass it into that?
Denis Lisov
@tanriol
I don't really understand what we'd win in this case...
Judson Lester
@nyarly
If we could have to_async_borrowing that worked with closures and functions, that simplifies that use case an awful lot.
Denis Lisov
@tanriol
...honestly, I'd prefer to make .to(|name: SomethingFromState, name2: SomethingElseFromState, ...| -> Result<...> async { ... }) work...
Judson Lester
@nyarly
Looking at it, I'm not sure that could happen on the Gotham side though.
Oh, certainly.
Hm. I'm not sure about the magical unpacking that's implied there.... Well, based on the hlist behavior of the borrowbag, though.
Naming that type seems rough though.
Denis Lisov
@tanriol
I'm not sure whether it could be made to work with multiple function arguments, but at some point I had it working with a tuple of these...
msrd0
@msrd0_gitlab
I guess that could actually work if we had an impl Handler<A, B, ...> for Fn(A, B, ...) -> Result<...> where A: BorrowFromState, ...
Judson Lester
@nyarly
That's not terrible. It winds up being a multi-impl trait, right? impl X for (FromState); impl X for (FromState, FromState) ...
I'd be okay with a private macro to do defs for 0..32 different FromStates
Denis Lisov
@tanriol
Not ideal either - matching types to names looked a bit weird in the tuple case if there can be a number of these in the tuple...
Judson Lester
@nyarly
Howso?
Denis Lisov
@tanriol
Also, there's a drawback: customizing the response if something is missing is either impossible or requires some more wrappers.
msrd0
@msrd0_gitlab
I guess we could also make name: Option<SomethingFromState> work
is there any benefit on using tuples over several method arguments?
Denis Lisov
@tanriol
Several method arguments is a bit more difficult to implement. Formally speaking, impl<A> Trait for Fn(A) and impl<A, B> Trait for Fn(A, B) do overlap because something can have both implementations. Requires a workaround I did not know at that point.
Judson Lester
@nyarly
I'd consider it a necessary evil, if it is necessary. But part of having a "define 32 impls" macro would be having the implementation of tuples from the state, and then handlers with variable arguments that get the tuple from the state and then pass to the handler.
msrd0
@msrd0_gitlab
oh I see, that's annoying
Judson Lester
@nyarly
Option<FromState> also might get unwieldy. You go from N implementations to 2^N.
We could say "Options follow required" and keep it to N^2.
msrd0
@msrd0_gitlab
one way to circumvent that would be to intruce a handler!(|name: A, name2: B| unimplemented!()) macro that expands to |foo: (A, B)| { let (name, name2) = foo; unimplemented!() }
Judson Lester
@nyarly
@tanriol I'm not sure I follow. How can a type be Fn(A) and Fn(A, B)?
msrd0
@msrd0_gitlab
it cannot but rustc thinks it's possible
I forgot how annoying traits can be sometimes
Judson Lester
@nyarly
Is there a bug for that?
Denis Lisov
@tanriol
It can in Nightly where it's possible to implement the Fn family traits manually.
Judson Lester
@nyarly
Curse you, "always stable"! :)
msrd0
@msrd0_gitlab
I don't think it'll be possible unless rust found a way for trait impls to exclude each other
Judson Lester
@nyarly
Wait, what if it were impl<A> Trait1 for Fn(A) and impl<A,B> Trait2 for Fn(A,B) etc?
Denis Lisov
@tanriol
You've just moved the problem to an overlap between Trait1 and Trait2 :-)
Judson Lester
@nyarly
Ah! Right.
Denis Lisov
@tanriol
I think I'm going to prototype a to_any or something like that and check what exactly explodes :-)
msrd0
@msrd0_gitlab
sounds good
Judson Lester
@nyarly
Sounds awesome, actually
Denis Lisov
@tanriol
By the way, does anyone know why we need the StateData derive?
Is it needed for some technical reason or is this just "being able to put anything Any + Send there can be confusing, let's restrict it"?
msrd0
@msrd0_gitlab
the docs call it a marker trait and its derive doesn't do anything so I assume it's the later
Denis Lisov
@tanriol
StaticResponseExtender also looks confusing... I have no idea how it's supposed to be used :-)
msrd0
@msrd0_gitlab
that is very confusing, I agree
what it does is configure a blanket response in case the whatever-it-is-implemented-on operation failed
so if you are matching /search?query=... but the client didn't send the query param, the extender is called (and, if using the derive, configures the response as a 400 Bad Request)
Denis Lisov
@tanriol
If we were doing breaking changes for the sake of user sanity, I'd suggest renaming it to actually mention failure :-)
(but I'll try to think whether there's a better API for that)
msrd0
@msrd0_gitlab
it's a terribly confusing way of error handling, and I only understood what it did after removing it from gotham and looking at the compile errors