Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate FromUriParam<Query> impls for some Option types #1420

Closed
wants to merge 1 commit into from
Closed

Generate FromUriParam<Query> impls for some Option types #1420

wants to merge 1 commit into from

Conversation

Alexendoo
Copy link
Contributor

I ran into this issue when trying to create next/prev page links while reusing existing optional parameters, something like

#[get("/route?<page>&<other>"]
fn route(page: usize, other: Option<String>) {
    let next = uri!(route, page + 1, other);
}

cc #998 (comment), it allows forms a and b for the added types

I tried creating a blanket impl for Option<T>, however that conflicts with the T -> Option<T> forwarding, so instead generate them for individual types using a macro

Option<&str>, Option<&RawStr> and Option<Cow<str>> are not included because they also generated conflicts with existing impls

@Alexendoo

This comment has been minimized.

@SergioBenitez
Copy link
Member

I'm generally in favor, but I'm wondering if instead we should do this:

diff --git a/core/http/src/uri/from_uri_param.rs b/core/http/src/uri/from_uri_param.rs
index 416d3946..8e774107 100644
--- a/core/http/src/uri/from_uri_param.rs
+++ b/core/http/src/uri/from_uri_param.rs
@@ -338,7 +338,7 @@ impl<'a, 'b> FromUriParam<uri::Path, &'a &'b str> for PathBuf {
 }
 
 /// A no cost conversion allowing any `T` to be used in place of an `Option<T>`.
-impl<P: UriPart, A, T: FromUriParam<P, A>> FromUriParam<P, A> for Option<T> {
+impl<A, T: FromUriParam<uri::Path, A>> FromUriParam<uri::Path, A> for Option<T> {
     type Target = T::Target;
 
     #[inline(always)]
@@ -348,7 +348,7 @@ impl<P: UriPart, A, T: FromUriParam<P, A>> FromUriParam<P, A> for Option<T> {
 }
 
 /// A no cost conversion allowing `T` to be used in place of an `Result<T, E>`.
-impl<P: UriPart, A, E, T: FromUriParam<P, A>> FromUriParam<P, A> for Result<T, E> {
+impl<A, E, T: FromUriParam<uri::Path, A>> FromUriParam<uri::Path, A> for Result<T, E> {
     type Target = T::Target;
 
     #[inline(always)]
@@ -356,3 +356,39 @@ impl<P: UriPart, A, E, T: FromUriParam<P, A>> FromUriParam<P, A> for Result<T, E
         T::from_uri_param(param)
     }
 }
+
+impl<A, T: FromUriParam<uri::Query, A>> FromUriParam<uri::Query, Option<A>> for Option<T> {
+    type Target = Option<T::Target>;
+
+    #[inline(always)]
+    fn from_uri_param(param: Option<A>) -> Self::Target {
+        param.map(|a| T::from_uri_param(a))
+    }
+}
+
+impl<A, E, T: FromUriParam<uri::Query, A>> FromUriParam<uri::Query, Option<A>> for Result<T, E> {
+    type Target = Option<T::Target>;
+
+    #[inline(always)]
+    fn from_uri_param(param: Option<A>) -> Self::Target {
+        param.map(|a| T::from_uri_param(a))
+    }
+}
+
+impl<A, E, T: FromUriParam<uri::Query, A>> FromUriParam<uri::Query, Result<A, E>> for Result<T, E> {
+    type Target = Result<T::Target, E>;
+
+    #[inline(always)]
+    fn from_uri_param(param: Result<A, E>) -> Self::Target {
+        param.map(|a| T::from_uri_param(a))
+    }
+}
+
+impl<A, E, T: FromUriParam<uri::Query, A>> FromUriParam<uri::Query, Result<A, E>> for Option<T> {
+    type Target = Result<T::Target, E>;
+
+    #[inline(always)]
+    fn from_uri_param(param: Result<A, E>) -> Self::Target {
+        param.map(|a| T::from_uri_param(a))
+    }
+}

@SergioBenitez SergioBenitez added the pr: closed This pull request was not merged label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: closed This pull request was not merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants