servant-docs: Fix docsWith. #124
Conversation
When adding extra info using using docsWith, the responses vanished from the output. This was due to combineAction being left-biased, and docsWith combining the extra info with the enpoint (in that order). Flipping combineAction solves this.
|
Hello Philipp and thanks for the PR! Upon seeing your PR, @jkarni realized that much of that "information merging logic" seems a bit fragile. We're wondering whether we could actually revamp some of that logic and find a smarter, sound & principled solution to this. So while your PR fixes the problem you were having, we're kind of afraid that someone else might encounter a similar dark corner tomorrow because of a similar problem. I would be much more comfortable having a lossless information merging logic, where we never drop any bit of information. tl;dr: thanks for fixing the issue you found, we might merge it as is or use this opportunity to try and find a more robust merging strategy =) |
|
Hi Alp, thanks for the quick reply. I'm actually having some very similar 'lopsided merging' code in one of my own projects, so I would be very interested if you can come up with something more robust, or to participate in a brainstorming phase. Of course, if I have a sudden flash of wit and find something, I'll let you know as well. Best, |
|
Would you mind just adding a little test for a minimal-version of the problem you encountered and which gets fixed by this PR? Just to make sure a future rewrite gets it right too. |
|
Sure, I'll do that. |
Make sure that no information is lost when providing additional information via docsWith. With the current left-biased implementation of combineAction, this can happen if the function arguments are in the wrong order.
|
The CI failure seems to be unrelated to the commit (apparently, the random package failed to install). |
|
I also gave the problem of making the merging more robust some thought. One thing that I think should be possible is to use indexed types, as in REPA (http://www.cse.unsw.edu.au/~benl/papers/guiding/guiding-Haskell2012-sub.pdf). You'd have two different Actions, let's call them Action A and Action B, where Action A is a complete Action containing all information derived from the API datatype. An ExtraInfo would contain Action B, which cannot have information about Responses. combining functions would have signatures Action A -> Action B -> Action A and Action B -> Action B -> Action B, but not Action A -> Action A -> Action A, so the type system would prevent you from accidentally discarding information by providing arguments in the wrong order, or by trying to combine two Actions with Reponses. It might be overkill, but servant already does a lot of magic on the type level, so maybe it fits in. What do you think? |
|
Re: the build failure, I've just restarted it because it seems to a random timeout or something rather than an actual problem. |
|
There you go. The build succeeds. So we're going to merge this and make a bugfix release in the upcoming days and then we're probably going to end up with a rewrite in 0.5. One of most promising directions here is to just rely on something like pandoc-type's Julian: good to merge? |
|
@alpmestan yup on my end. @kantp hi! and thanks! |
|
Thanks @kantp! |
|
You're very welcome, thanks for handling this so quickly, and for writing the library in the first place! @jkarni Hi there, hope to see you at the next meetup! |
|
Cherry picking this. |
Hi,
thanks for this nice library :)
I noticed that the enpoints vanished from the documentation when adding extra info via docsWith. This pull-request fixes that.
Best,
Philipp