Add getAccessTokenWith

[rbtcollins]

This more flexible API:

• returns an Either, permitting custom error handling (e.g. polling
  until the user has authorized the token, which CLIs need to do)
• Permit different ways of signing access token requests, allowing
  use with sites that use body encoding (OAuth 1.0 section 5.2 point
  2) instead of Authorization headers.

[snoyberg]

What's the purpose in adding a new function that takes an argument that it ignores?

[rbtcollins]

See the note about an API break - I didn't want to break the public API, and this needed the same type as addAuthHeader which is the other implementation of adding authentication to a request, and addAuthHeader does use the first parameter.

[snoyberg]

Why does it need the same type as addAuthHeader?

[rbtcollins]

So one can use either function depending on which way you want the request signature attached to the request. Perhaps theres a better way to do this :) - I'm new to Haskell and very happy to be guided.

[snoyberg]

I'm still not following what the motivation is, or what the question is. Why not just leave off that parameter if it's not used?

[rbtcollins]

I'd be happy to. I don't know how to do that and still support both of these as valid calls:

a) convert a temporary token to an access token on a site using section 5.2.2 POST body encoding (e.g. Launchpad only accepts this form):
mytoken <- getAccessToken'' addAuthBody id some_oauth temp_credential manager

b) convert a temporary token to an access token on a site using section 5.2.1 Authorisation header (e.g. Github)
mytoken <- getAccessToken'' addAuthHeader id some_oauth temp_credential manager

If I remove the unused parameter from addAuthBody then the first will get a type error. If I change the type for the parameter to mytoken then I'll get a type error in b) using it with addAuthHeader. the parameter that you're asking about is used with addAuthHeader, its just a not used for addAuthBody.

If I change the signature of addAuthHeader, or add an addAuthHeader' then I can make it the same as addAuthBoth needs, by pushing the realm logic down into the auth attachment helper (which is where I think it belongs btw, just like I said, didn't want to break the API for addAuthHeader).

[snoyberg]

Now I understand what you're saying, I didn't realize you were using this as the parameter to getAccessToken'' (you should know that I only recently took over maintainership of this library, and have actually never used it myself). OK, that makes sense. The way you could do it is const addAuthBody... but the way you've written it now is probably better. I'd recommend:

1. Making the first parameter a simple a instead of hard-coding to BS.ByteString.
2. Adding a Haddock comment explaining that the argument is not used, and that it's present simply to give the same type as addAuthHeader.

[rbtcollins]

1 is easy, will do. I'm not sure what 2 entails, is that that ^ - stuff ?

I've a revised patch that has the getAccessTokenWith as requested. I'm also adding in a change to the return type there to return an Either, which is useful for handling some errors (e.g. it enables polling). Should have it all pushed up in an hour or so.

[snoyberg]

Yes, more information on Haddock is available at: https://www.haskell.org/haddock/doc/html/ch03s08.html

[snoyberg]

I just pushed a commit to a separate branch: 6b2be36. Can you review this? If you're happy with it, then I'll merge it into master (including your changes) and release to Hackage.

Thanks for incorporating my feedback.

[rbtcollins]

Looks totally nice. I pushed up a variation here before I noticed your new branch; all it did was ' quote the material in the addAuthBbody comment.

[rbtcollins]

Thanks for merging this!

[snoyberg]

Certainly! Thanks for the contribution.

On Fri, Feb 13, 2015, 11:52 AM rbtcollins [email protected] wrote:

Thanks for merging this!

—
Reply to this email directly or view it on GitHub
#42 (comment).