Add initial Markdown support

[strzibny]

This is WIP change for Markdown support.

[strzibny]

Maybe we should call this .md() instead of .markdown() to align with serpapi/serpapi-python#39

[trusche]

Why separate format and output arguments?

[trusche]

Maybe we should call this .md() instead of .markdown() to align with serpapi/serpapi-python#39

Personally I prefer markdown() over md()

[strzibny]

Thanks @trusche for having a look. You are right on search_archive, we should cleanly just pass the extension. I fixed it.

Personally I prefer markdown() over md()

Well, that's how I started as well :) However, consistency among our clients is more important I think (already in Python and PHP libs) and I think there is a tiny advantage of matching the extension.

[trusche]

Ruby and rails offer a lot of syntactic sugar, we could at least create an alias?

[strzibny]

I was thinking about it, but on the other hand I want us to be direct in docs and want to send people to one method everywhere. So creating an alias just on its own doesn't feel that good to me.

json, html, md are all shortcuts/abbreviations and in that sense it just works in uniform. I think that in the end uniformity among the abbreviations and across the libraries is just more important. They also directly suggest the extension in their meaning.

Do you think it's still worth it?

[pulkitchowdry]

@strzibny Thanks for the efforts. Functionality wise everything looks good to me 👍

One minor thing I observed (not due to code changes as part of this PR) is that when I pass output: "md" or output: "html" as a parameter in my SerpApi params then my output response for search method is broken with JSON parse related errors (since we default decoder to json. For any other values, for example output: "markdown" we throw an error instead of a broken response. Should we also throw an error for this? maybe we can check under a new issue.

I feel it would be good to mention some sample code in our Readme doc. Something like below where we are differentiating the three,

# For JSON output
result_json     = serpapi_client.search

# For HTML output
result_html     = serpapi_client.html

# For Markdown output
result_md       = serpapi_client.md

Note - I observe that our python library supports output in params so maybe in the future we can also support it in Ruby.

[pulkitchowdry]

LGTM 👍

[strzibny]

Thanks @pulkitchowdry, you are right, we should have already been choosing the right decoder for HTML. I included it in this change.

[pulkitchowdry]

@strzibny Thanks for the changes. Output parameter is working now!

One issue I observed is that when we do not pass output = "html" and call serpapi_client.html(q: "coffee") then the response is json and not in html format. Should this be like below?

Suggested change

	get('/search', :html, params)
	get('/search.html', :html, params)

Other tests I performed are mismatch between output parameter and the method called and it works properly. Its response is similar to SerpApi cURL response - for example if output parameter is json but we call .html then we respond with json which is fine as the output parameter is taking priority like the main SerpApi.

[strzibny]

@pulkitchowdry Great find. I realized we need to fix it a bit better. If we only use search.html route we still have a case where client.md(output: "json") would use the Markdown decoder and return JSON as a raw string rather than a Ruby Hash.

I made a change to always return hash on output=json, string otherwise, basically doing what we do at the server side since /search.md?q=coffee&output=json gives output the preference. So I am matching this behavior.

[trusche]

I think the md, json and html methods should simply not support the output parameters - they're basically shortcuts to search. Let's remove it from the documentation and ignore any passed-in output for these methods? Only search() should support it, with :json as the default.

[strzibny]

I think you are right @trusche it also overly complicates 'what should we do'. I still kept CONTENT_TYPE_DECODERS because with this change we can properly show errors that server returns as JSON. Wdyt?

[trusche]

I still don't get why we need separate decoder and output arguments. Would it ever be used with different formats?

[strzibny]

Would you be more happy with something like this?

def response_decoder(response, default)
  output = response.headers['Content-Type'].to_s.split(%r{[/;]})[1]
  output = 'md' if output == 'markdown'
  OUTPUT_DECODERS.fetch(output, default)
end

Not sure if I understand what you are asking but running client.md(...) will have to process JSON in case of errors.

[strzibny]

And in another case client.send(:get, '/invalid', :json, {}) is returning HTML with text/html content type. So the idea is to process responses by the content type we actually get from serpapi.com