WIP: extend the embed functionality #112

Draft
Zankaria wants to merge 20 commits from embedding-v2 into config
Owner
No description provided.
@ -0,0 +43,4 @@
* @return string Returns the body of the response.
* @throws RuntimeException Throws on IO error.
*/
public function requestGet(string $endpoint, ?array $data, ?array $headers, int $timeout = 0): string {
First-time contributor

Add explicit null to headers or it will have to have every captcha file

Add explicit null to headers or it will have to have every captcha file
Zankaria marked this conversation as resolved
@ -0,0 +40,4 @@
],
$this->provider_timeout
);
$json = \json_decode($body, null, 512, \JSON_THROW_ON_ERROR);
First-time contributor

This returns an object causing everything else to fail

This returns an object causing everything else to fail
Zankaria marked this conversation as resolved
@ -0,0 +55,4 @@
}
}
$this->cache->set("oembed_embedder_$url$provider_url", $ret, $cache_timeout);
First-time contributor

There's two cache keys. One at line 30 and another one at 58. both are differently

There's two cache keys. One at line 30 and another one at 58. both are differently
Zankaria marked this conversation as resolved
@ -0,0 +61,4 @@
$resp = new OembedResponse();
$resp->title = $ret['title'];
$resp->thumbnail_url = $ret['thumbnail_url'];
return $ret;
First-time contributor

The return must be $resp

The return must be $resp
Zankaria marked this conversation as resolved
@ -0,0 +24,4 @@
}
private function make_tmp_file(): string {
$ret = \tempnam($this->tmp_dir, self::TMP_FILE_PREFIX);
First-time contributor

$this->tmp_dir is never initialized causing an error

$this->tmp_dir is never initialized causing an error
@ -0,0 +61,4 @@
$type = $embed_entry['type'];
if ($type === 'oembed') {
$thumbnail_url_fallback = $embed_entry['thumbnail_url_fallback'] ?? null;
First-time contributor

eh, you could create a generic fallback outside of the array instead of putting a fallback for each embedding. customization is cool and all, but will you really create images for all embeds?

eh, you could create a generic fallback outside of the array instead of putting a fallback for each embedding. customization is cool and all, but will you really create images for all embeds?
@ -0,0 +104,4 @@
public function getEmbedThumb(string $url, int $entry_index): ?array {
$ret = $this->extractThumb($url, $entry_index);
list($thumbnail_url, $thumbnail_url_fallback) = $ret;
if (!isset($thumbnail_url, $thumbnail_url_fallback)) {
First-time contributor

you could remove fallback from here. if an embed doesnt have a fallback it simply fail without ever trying to run. and you already check for a fallback at line 121

you could remove fallback from here. if an embed doesnt have a fallback it simply fail without ever trying to run. and you already check for a fallback at line 121
@ -0,0 +124,4 @@
return [ $thumbnail_url_fallback, false ];
}
public function renderEmbed(string $url, int $entry_index, string $thumbnail_path): string {
First-time contributor

in the current architecture this doesnt fit here. we would need to initiate embed service in display just to do a replace

in the current architecture this doesnt fit here. we would need to initiate embed service in display just to do a replace
@ -0,0 +44,4 @@
throw new \RuntimeException("Could not open temporary file $tmp_file for read/write");
}
$ret = $this->http->requestGetInto($thumbnail_url, null, $fd, $this->thumb_download_timeout);
First-time contributor

$this->http is never initialized as well

$this->http is never initialized as well
@ -0,0 +115,4 @@
$opt = (\PHP_MAJOR_VERSION >= 8 && \PHP_MINOR_VERSION >= 2) ? \CURLOPT_XFERINFOFUNCTION : \CURLOPT_PROGRESSFUNCTION;
\curl_setopt_array($this->inner, [
\CURLOPT_NOPROGRESS => false,
$opt => fn($res, $next_dl, $dl, $next_up, $up) => (int)($dl <= $this->max_file_size),
First-time contributor

only got the download to work by changing (int)($dl <= $this->max_file_size) to (int)($dl > $this->max_file_size)

only got the download to work by changing `(int)($dl <= $this->max_file_size)` to `(int)($dl > $this->max_file_size)`
This pull request has changes conflicting with the target branch.
  • inc/init.php
  • post.php
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin embedding-v2:embedding-v2
git switch embedding-v2

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch config
git merge --no-ff embedding-v2
git switch embedding-v2
git rebase config
git switch config
git merge --ff-only embedding-v2
git switch embedding-v2
git rebase config
git switch config
git merge --no-ff embedding-v2
git switch config
git merge --squash embedding-v2
git switch config
git merge --ff-only embedding-v2
git switch config
git merge embedding-v2
git push origin config
Sign in to join this conversation.
No reviewers
No labels
Bug
Proposal
Request
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
leftypol/leftypol!112
No description provided.