fix: Add DownloadContent to support downloads of any size

[sheeeng]

Fix#3810.
Fix#2480.

[gmlewis]

My biggest concern is that we now have:

• DownloadContent
• DownloadContents
• DownloadContentsWithMeta
• GetContent
• GetContents

How is a user supposed to understand which one to choose?

[sheeeng]

My biggest concern is that we now have:

• DownloadContent
• DownloadContents
• DownloadContentsWithMeta
• GetContent
• GetContents

How is a user supposed to understand which one to choose?

Good point. I will think about it.

[sheeeng]

• DownloadContents
• DownloadContentsWithMeta

These two are existing functions. Do we want to expose DownloadContents only for use?

[sheeeng]

Do we want to expose DownloadContents only for use?

The DownloadContentsWithMeta function and its references were deleted. Is that preferred?

[gmlewis]

The DownloadContentsWithMeta function and its references were deleted. Is that preferred?

Well, it seems we have a number of people interested in this topic based on the conversation in #2480 and #3810.

I would really like to get some feedback from the individuals who care about these endpoints before we start changing it dramatically, and the only way I can think of doing that is to start listing GitHub logins that commented, unfortunately.

cc: Please provide your feedback on how these endpoints should be handled:

• @ran-arigur
• @abatilo
• @zaataylor
• @petersondmg
• @githoober
• @JayDubyaEey

[ran-arigur]

My biggest concern is that we now have:

• DownloadContent
• DownloadContents
• DownloadContentsWithMeta
• GetContent
• GetContents

How is a user supposed to understand which one to choose?

(Full disclosure: Adding DownloadContent was my suggestion, so I can't independently say whether it will make sense to other people.)

FWIW, those methods are not all on the same type. Grouping it a bit differently:

• on RepositoriesService:
  • GetContents(...)
    • is genuinely a bit confusing, but I don't think we should change it, because it maps pretty directly to an underlying API call [doc link] and what makes it confusing is that that API is confusing.
    • returns either a single RepositoryContent or a slice of RepositoryContents, depending on whether the path is a file or a directory.
  • DownloadContents(...), DownloadContentsWithMeta(...)
    • convenience wrappers around GetContents(...) + DownloadContent().
    • behave identically, but DownloadContentsWithMeta(...) returns more information. I assume that DownloadContents(...) was created first, and then DownloadContentsWithMeta(...) was added later because there was no backward-compatible way to have DownloadContents(...) return the additional information.
• on RepositoryContent (as returned by RepositoriesService.GetContents(...)):
  • GetContent()
    • returns the file content, if already available on the RepositoryContent. handles Base64-decoding if necessary.
    • doesn't handle cases where the RepositoryContent doesn't contain the file content, e.g. if the file is >1MB (in which case it returns an error) or the RepositoryContent came from listing a directory (in which case it returns the empty string even if the file is non-empty).
  • DownloadContent()
    • equivalent to GetContent(), except that it works even if the RepositoryContent doesn't contain the file content: in that case it fetches it using the RepositoryContent's DownloadURL.

I think that, with reasonable comments on each, users will be able to find a method that does what they need, and ignore the rest.

[sheeeng]

Setting this PR as draft as waiting for more feedback.

[gmlewis]

Closing, as this was resolved by #4153.