How to render readme content as HTML?

[Play-Maker]

/* func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string, opt *RepositoryContentGetOptions) (*RepositoryContent, *Response, error) */
this func cannot set http header "application/vnd.github.VERSION.html+json".

[Play-Maker]

May be we can add some elements into struct RepositoryContentGetOptions so that we can set http header or body.

[dmitshur]

It seems that RepositoryContentGetOptions struct is used in many places, I'm not sure if this new struct field would apply to all of them. We should take that into account.

Other than that, adding a struct field makes sense. We've done something similar for #481, that can serve as an example.

[kshitij10496]

I would like to give this a try.

[gmlewis]

Invitation sent. Once you accept the invitation, @kshitij10496, we can assign this issue to you.
Thank you!

[kshitij10496]

I would like to share my research on the custom media types supported by the Repo Contents endpoints:

Media Type	Header	Response	Currently Supported
html	application/vnd.github.v3.html	Response is raw HTML	No
raw	application/vnd.github.v3.raw	Response is raw text	No
object	application/vnd.github.v3.object	This is the default base64 encoded JSON response which we currently support	Yes

Hence, I think we should add support for the remaining media types, namely html and raw.

[kshitij10496]

I have thought of 3 implementations which we can use for supporting different media types for repository contents. I am having a difficulty understanding the which approach is most suitable for our case. Thus, I would like to share them before opening up a PR.

Set up

Based on the method used in #481 and #767, I created a new type to represent the above-mentioned custom media types as:

type RepositoryContentMediaType uint8

const (
	Object RepositoryContentMediaType = iota // since this is the default
	Raw
	HTML
)

---

Approach 01: Adding a new parameter to the function signature of GetReadme method.

This approach is similar to the one used in #767 where we modified the function signature whereby the user needs to pass in a new argument related to the desired Media Type. Moreover, the signature of these functions is similar to GetArchiveLink with the archiveFormat type being replaced by the RepositoryContentMediaType.

Pros	Cons
Separation between API Parameters and Media Types	Not Backwards compatible with current API. Need to implement new methods specific to custom media types.

Usage example:

// Get README as HTML
client.Repositories.GetReadme(context.Background(), "owner", "repo",
	github.HTML, RepositoryContentParameters{ref: "master"})

---

Approach 02: Adding a MediaType field to RepositoryContentGetOptions struct.

As suggested by @shurcooL above and @willnorris in #6, I attempted to create a new field to the struct dealing with handling options as:

type RepositoryContentGetOptions struct {
	Ref       string `url:"ref,omitempty"`
	MediaType RepositoryContentMediaType
}

However, the problem I am facing here is with the expected behaviour of addOptions function used for generating the API endpoint. The function expects the argument passed to it to be a struct with "url" tags. Thus, I used an anonymous struct which is then passed onto addOptions for processing.

params := struct{
    Ref string `url:"ref,omitempty"`
}{
    opt.Ref
}

Usage example:

// Get README as HTML
client.Repositories.GetReadmeCustom(context.Background(), "owner", "repo",
	&RepositoryContentGetOptions{ref: "master", MediaType: HTML})

Pros	Cons
Backwards compatible with the current API of various methods (except GetArchiveLink method)	We will need to duplicate modification if a new Parameter is added by GitHub to the endpoints.

---

Approach 03: Separating the endpoint Parameters from the Media Types

In this approach, I create new types for Parameters(RepositoryContentParameters) and Media Types(RepositoryContentMediaType). Using these types, I compose the RepositoryContentGetOptions as:

type RepositoryContentGetOptions struct {
	Params    RepositoryContentParameters
	MediaType RepositoryContentMediaType
}

Usage example:

// Get README as HTML
client.Repositories.GetReadmeCustom(context.Background(), "owner", "repo",
	&RepositoryContentGetOptions{Params: RepositoryContentParameters{ref: "master"},
		MediaType: HTML})

Pros	Cons
Design by composition	API becomes too dense and difficult to read
Ease in extending support for future Endpoint Parameters

[dmitshur]

Thanks for outlining the approaches so they're easier to compare and discuss! Indeed, it's not easy to tell which is the best one.

A part of what makes it harder is that RepositoryContentGetOptions is reused across many different methods, so it's hard to customize for GetReadme needs without affecting others.

I think it'd help us make a decision if we find out how many other places will need support for specifying custom media type. So far, we only had 2 (#481 and #767). This is the 3rd. Is there only a couple more? If there's more than 10 methods that'll be affected similarly, we might need to consider a different approach.

If there's no more than 10, I'd probably say it's least bad to go with approach 3 and follow the pattern we set out previously of adding a new method with a Raw suffix that takes an extra parameter specifying the content-type.

[kshitij10496]

I think it'd help us make a decision if we find out how many other places will need support for specifying custom media type.

After reading the GitHub documentation here and grepping through the code, I found the following:

• Methods which should support custom media types for Repository Contents API:

  • GetReadme
  • GetContents and hence,
  • DownloadContents

• GetArchiveLink method is the only place where the ref parameter(abstracted by RepositoryContentGetOptions) is used but which doesn't support any custom media type.

However, if we are discussing support for all the different possible custom media types supported by GitHub API across the entire client, this exhaustive list suggests that we don't have to modify many services but there are more than 10 endpoints which support custom mime types.

I hope this information helps us decide our way forward.

[kshitij10496]

@shurcooL Have you had time to think about how should we proceed from here?

Apart from that, I had a suggestion regarding the naming convention of methods for supporting custom media types.
How about we name them as GetReadmeCustom instead of GetReadmeHTML?

Implement a generic method which provides support for media types for each endpoint has 2 advantages:

1. The number of methods we need to implement for supporting mime types for an endpoint is independent of the number of actually supported custom media types.
2. Extending support for new media types in the future will be easier.

For example, in case of GetReadme, a single method GetReadmeCustom can support all HTML, raw and object media types.

[dmitshur]

@kshitij10496 Sorry, I don't have a lot of bandwidth to dedicate to thinking about this issue at this time. See if you can make progress without blocking on me, and hopefully other maintainers/contributors can chime in as needed.

[wesleimp]

@gmlewis Can this issue be closed?

[gmlewis]

Looking at the history of PRs for this issue, I'm going to close it.