Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added Youtube extension #217

Merged
merged 14 commits into from
Mar 17, 2024
Merged

added Youtube extension #217

merged 14 commits into from
Mar 17, 2024

Conversation

kevinpapst
Copy link
Collaborator

@kevinpapst kevinpapst commented Mar 12, 2024

See #216

  • Moves the YouTube extension to the official repository, to prevent multiple forks
  • Removed my other two extensions, as they do not work any longer
  • Commits from the currently linked fork should be added in a second PR

Dear Reviewer
I do not have a running FreshRSS installation for testing. The 8+ year old codebase was upgraded blindly to latest FreshRSS and "modern" PHP style, especially the code regarding user configuration handling needs testing. In other words: install before merge 😁

@kevinpapst kevinpapst marked this pull request as draft March 12, 2024 21:03
@kevinpapst kevinpapst marked this pull request as ready for review March 12, 2024 21:59
@Alkarex
Copy link
Member

Alkarex commented Mar 14, 2024

Help to test welcome (I am not a user myself), and optionally to finalise the light code upgrade (e.g. [ ] instead of array(), <?= $x ?> instead of <?php echo $x; ?>, ...) but that is not urgent.

Ping @ImAReplicant
Let's merge the latest changes if there is any missing from https://framagit.org/ImAReplicant/freshrss-youtube

@pax0707
Copy link

pax0707 commented Mar 15, 2024

Updated it yesterday.
Didn't notice any issues.

@ImAReplicant
Copy link
Contributor

Hello,
I've been browsing the various topics about the youtube extension and there seems to be a misunderstanding about the fork I made.

I never wanted to steal @kevinpapst's work, in fact I had opened an issue to ask if I could integrate the fork (#133 (comment)). I thought at the time that the extension was no longer maintained.
I may have done it clumsily and I really apologize for that.

The changes I've made concern peertube videos, because without my changes peertube videos wouldn't be integrated.

In extension.php file,
line 136 :
if (preg_match('#^https?://www\.youtube\.com/watch\?v=|/videos/watch/[0-9a-f-]{36}$#', $link) !== 1) {
to
if (preg_match('#^https?://www\.youtube\.com/watch\?v=|/w/[0-9a-zA-Z]{22}$#', $link) !== 1) {

line 177 :
$url = str_replace('/watch', '/embed', $link);
to
$url = str_replace('/w', '/videos/embed', $link);

As for the rest of the changes, it was an attempt to modify the display of the peertube description, but it doesn't seem to work anymore. Indeed, the description is displayed in a rather strange way:

Capture d’écran du 2024-03-16 10-45-30

Sorry again for this misunderstanding and the problems it has caused.

@kevinpapst
Copy link
Collaborator Author

Thanks for your explanation @ImAReplicant

From my end this is ready for merge, as @pax0707 confirmed that the current version works.

Let's do the Peertube (how many people use that?) integration either in its own extension or at least in second PR.

@Alkarex
Copy link
Member

Alkarex commented Mar 17, 2024

Could you please update https://github.com/FreshRSS/Extensions/blob/master/repositories.json accordingly?

@kevinpapst
Copy link
Collaborator Author

Done

@Alkarex Alkarex merged commit ad2d0ee into FreshRSS:master Mar 17, 2024
1 check passed
@kevinpapst kevinpapst deleted the youtube branch March 17, 2024 22:33
@kevinpapst
Copy link
Collaborator Author

Thanks for accepting the extension!

MentalFS added a commit to MentalFS/linuxserver-docker-mods that referenced this pull request Mar 18, 2024
@Alkarex
Copy link
Member

Alkarex commented Mar 19, 2024

#222

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants