[WIP] Add markdown API - Githubissues

octokit / go-octokit

Simple Go wrapper for the GitHub API

https://github.com/octokit/go-octokit

MIT License

258 stars 79 forks source link

[WIP] Add markdown API #112

Open obsc opened 9 years ago

obsc commented 9 years ago

Added functionality for https://developer.github.com/v3/markdown/

@pengwynn It seems like the markdown API is one of the few (only?) API endpoints that return html rather than json. However, there is no support for encoding and decoding text/html;charset=utf-8. This is due to the fact that go-sawyer was only initialized with an encoder and decoder for json: https://github.com/jingweno/go-sawyer/blob/master/sawyer.go#L82-L89

At the moment, the workaround I implemented is kind of hacky: I get the raw response back from the server and then directly parse it into a string.

Additionally, encoding a string with setBody defaults to using json encoding as well. This results in strings getting encoded with two extra quotation marks: testBody(t, r, "\"Hello world github/linguist#1 **cool**, and #1!\"\n") should be testBody(t, r, "Hello world github/linguist#1 **cool**, and #1!\n")

What are your thoughts about this?

Edit: Just realized I accidentally used windows line breaks

feiranchen commented 9 years ago

I don't have better ideas for html parsing either, cuz it doesn't seem to fall in the scope of sawyer, nor is there common use cases.

However, it seems like you are just checking for raw text, maybe a shorter sample text will just do for testing purposes? Especially since we are testing on client side only?

Just my 2 cents, feel free to comment!

obsc commented 9 years ago

Hmm, a shorter sample text could make the tests simpler. But the main issue lies with decoding the response. We don't necessarily need html parsing per se, but might need a better way to parse raw text from the response. Right now, I have to use ioutil to transform the body of the response into a bitarray to work with.

pengwynn commented 9 years ago

It seems like the markdown API is one of the few (only?) API endpoints that return html rather than json. However, there is no support for encoding and decoding text/html;charset=utf-8. This is due to the fact that go-sawyer was only initialized with an encoder and decoder for json:

This method is indeed an outlier. We have other cases for returning non-JSON, but I don't think we have anywhere where we're POST-ing non-JSON.

Have you tried sending an Accept header?

feiranchen commented 9 years ago

Oops, I pushed before I saw your comment. However I just tried and an Accept header didn't help. Also, I looked into getBody and it can't be used off-the-shelf mainly because result.Response.Body is a bit stream that needs to be decoded. That means using it doesn't make Rene's version any cleaner, I believe.

I've tried to address the problems Rene brought up. In the commit above, I did the following:

coded an encoder for raw text request and the two extra quotation marks went away.
re-factored the "decoder" for raw response into method createResponseRaw. However there probably exists a clear approach.
expanded Render into methods Render and RenderRaw for clarity

Please take a look at this version and let me know what you think!