warp icon indicating copy to clipboard operation
warp copied to clipboard

Document that filters::query::query uses serde_urlencoded, and add example for postprocessing filters::query::raw

Open jcaesar opened this issue 5 years ago • 8 comments
trafficstars

serde_urlencoded is used for to deserialize query strings, and hence not all structs that are Deserialize can be deserialized to. I think it would be neat to document that fact and also added an example that shows how to use a different deserialization function instead.

(I didn't want to add serde_qs as a dev-dependency just for this, so I fudged it.)

What do you think?

jcaesar avatar Aug 08 '20 14:08 jcaesar

Yes this is a great idea, thanks! I wonder, what if instead of "promising" to use serde_urlencoded, we just say that currently the decoder strictly follows the URL spec, and so cannot decode lists etc.

seanmonstar avatar Aug 08 '20 15:08 seanmonstar

currently the decoder strictly follows the URL spec

Hm, I'll rephrase it like that. But I do wonder whether it's accurate, or at least helpful. On the one hand side, the URL spec allows duplicated keys (okay, it'd be odd to expect that to work with rust structs structs), on the other hand side, serde_urlencoded supports tagged/untagged enums, which I had not expected to work at all (but only if all values are String). Then again, mentioning serde_urlencoded does not help that much, since these things aren't documented.

jcaesar avatar Aug 09 '20 01:08 jcaesar

Maybe explicitly stating what can be decoded into is a good idea?

jcaesar avatar Aug 09 '20 04:08 jcaesar

sorry! missed this one and this has since been addressed by #843

jxs avatar Jun 05 '21 16:06 jxs

I don't see how #843 documents what limitations the limitations of query parameter deserialization. I wouldn't consider this addressed. But I do feel like I'm being pedantic. In the off-case that you want me to renovate this PR, please do tell.

jcaesar avatar Jun 06 '21 06:06 jcaesar

Oh sorry! And no, not pedantic at all, if you want to rebase your PR against master and add the limitations of parameter deserialization that would be great!

jxs avatar Jun 07 '21 12:06 jxs

I've rebased on master and reworded it so the documentation doesn't leak any implementation details.

jcaesar avatar Aug 25 '21 13:08 jcaesar

Ah, if you're asking, I should probably spell it out. Parameters. But I'll change it to FooQuery to match the other example.

jcaesar avatar Sep 03 '21 00:09 jcaesar