friendica/friendica-addons
9
4
Fork 16
Code Issues 3 Pull requests 2 Releases 16 Wiki Activity

Updated URL Replace addon #1483

Merged
MrPetovan merged 18 commits from toddy/friendica-addons:2024.03-rc into 2024.03-rc 2024-03-19 18:00:46 +01:00
Conversation 4 Commits 18 Files changed 6 +49 -12
2 changed files with 49 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit c6daf2381c - Show all commits

3
url_replace/templates/admin.tpl
View file

@ -1,5 +1,8 @@
{{include file="field_checkbox.tpl" field=$nitter_server_enabled}}
{{include file="field_input.tpl" field=$nitter_server}}
{{include file="field_checkbox.tpl" field=$invidious_server_enabled}}
{{include file="field_input.tpl" field=$invidious_server}}
{{include file="field_checkbox.tpl" field=$proxigram_server_enabled}}
{{include file="field_input.tpl" field=$proxigram_server}}
{{include file="field_textarea.tpl" field=$twelvefeet_sites}}

58
url_replace/url_replace.php
View file

@ -21,8 +21,11 @@ function url_replace_install()
*/
function url_replace_addon_admin_post()
{
DI::config()->set('url_replace', 'nitter_server_enabled', !empty($_POST['nitter_server_enabled']));
DI::config()->set('url_replace', 'nitter_server', rtrim(trim($_POST['nitter_server']), '/'));
DI::config()->set('url_replace', 'invidious_server_enabled', !empty($_POST['invidious_server_enabled']));
DI::config()->set('url_replace', 'invidious_server', rtrim(trim($_POST['invidious_server']), '/'));
DI::config()->set('url_replace', 'proxigram_server_enabled', !empty($_POST['proxigram_server_enabled']));
DI::config()->set('url_replace', 'proxigram_server', rtrim(trim($_POST['proxigram_server']), '/'));
// Convert twelvefeet_sites into an array before setting the new value
$twelvefeet_sites = explode(PHP_EOL, $_POST['twelvefeet_sites']);
@ -46,13 +49,21 @@ function url_replace_addon_admin_post()
*/
function url_replace_addon_admin(string &$o)
{
$nitter_server_enabled = DI::config()->get('url_replace', 'nitter_server_enabled', true);
$nitter_server = DI::config()->get('url_replace', 'nitter_server');
$invidious_server_enabled = DI::config()->get('url_replace', 'invidious_server_enabled', true);
$invidious_server = DI::config()->get('url_replace', 'invidious_server');
$proxigram_server_enabled = DI::config()->get('url_replace', 'proxigram_server_enabled', true);
$proxigram_server = DI::config()->get('url_replace', 'proxigram_server');
$twelvefeet_sites = implode(PHP_EOL, DI::config()->get('url_replace', 'twelvefeet_sites') ?? []);
$t = Renderer::getMarkupTemplate('admin.tpl', 'addon/url_replace/');
$o = Renderer::replaceMacros($t, [
'$nitter_server_enabled' => [
'nitter_server_enabled',
DI::l10n()->t('Replace links to X.'),
$nitter_server_enabled,
],
'$nitter_server' => [
'nitter_server',
DI::l10n()->t('Nitter server'),
@ -61,6 +72,11 @@ function url_replace_addon_admin(string &$o)
null,
'placeholder="https://nitter.net"',
],
'$invidious_server_enabled' => [
'invidious_server_enabled',
DI::l10n()->t('Replace links to YouTube.'),
$invidious_server_enabled,
],
'$invidious_server' => [
'invidious_server',
DI::l10n()->t('Invidious server'),
@ -69,6 +85,11 @@ function url_replace_addon_admin(string &$o)
null,
'placeholder="https://yewtu.be"',
],
'$proxigram_server_enabled' => [
'proxigram_server_enabled',
DI::l10n()->t('Replace links to Instagram.'),
$proxigram_server_enabled,
],
toddy marked this conversation as resolved Outdated
MrPetovan commented 2024-03-16 18:26:00 +01:00
Outdated
Review
Copy link

Code standards: Please wrap operators with spaces.

Code standards: Please wrap operators with spaces.
toddy commented 2024-03-16 20:23:31 +01:00
Outdated
Review
Copy link

Strange, I did run bin/dev/php-cs-fixer/vendor/bin/php-cs-fixer for the file. However, I've seen that you've done some reformatting of my code as well in commit eeb783d71d -- I'm fine with that, but just out of curiosity: Could you please point me to the canonical configuration settings for PHP-CS-Fixer? I thought it's .php_cs.dist in the root directory, but when using that, my code does not get reformatted with spaces between operators.

Strange, I did run `bin/dev/php-cs-fixer/vendor/bin/php-cs-fixer` for the file. However, I've seen that you've done some reformatting of my code as well in commit https://git.friendi.ca/friendica/friendica-addons/commit/eeb783d71d713df23038102048284c6b170634cb -- I'm fine with that, but just out of curiosity: Could you please point me to the canonical configuration settings for PHP-CS-Fixer? I thought it's `.php_cs.dist` in the root directory, but when using that, my code does not get reformatted with spaces between operators.
MrPetovan commented 2024-03-19 17:54:08 +01:00
Outdated
Review
Copy link

Sorry about that, we're following the PSR-12 coding standards: https://www.php-fig.org/psr/psr-12/#62-binary-operators

I don't use PHP CS Fixer myself so I'd be hard-pressed to verify the configuration is correct.

Sorry about that, we're following the PSR-12 coding standards: https://www.php-fig.org/psr/psr-12/#62-binary-operators I don't use PHP CS Fixer myself so I'd be hard-pressed to verify the configuration is correct.
'$proxigram_server' => [
'proxigram_server',
DI::l10n()->t('Proxigram server'),
@ -95,36 +116,49 @@ function url_replace_addon_admin(string &$o)
function url_replace_render(array &$b)
{
$replaced = false;
$replacements = [];
$nitter_server = DI::config()->get('url_replace', 'nitter_server');
if (empty($nitter_server)) {
$nitter_server = 'https://nitter.net';
}
$nitter_server_enabled = DI::config()->get('url_replace', 'nitter_server_enabled', true);
if ($nitter_server_enabled) {
$replacements = array_merge($replacements, [
'https://mobile.twitter.com' => $nitter_server,
'https://twitter.com' => $nitter_server,
'https://mobile.x.com' => $nitter_server,
'https://x.com' => $nitter_server,
]);
}
toddy marked this conversation as resolved Outdated
heluecht commented 2024-03-14 06:46:53 +01:00
Outdated
Review
Copy link

Could you use the third parameter of the get function for the default value? Also it would be nice to use constants for the default URL. See for example the inviduous addon for that.

Could you use the third parameter of the `get` function for the default value? Also it would be nice to use constants for the default URL. See for example the inviduous addon for that.
toddy commented 2024-03-14 08:49:23 +01:00
Outdated
Review
Copy link

Thanks for your review. Unfotunately, the third parameter of get() does not work in this case, I've tried that. When the addon is freshly installed, there is no configuration in the database. In that case, DI::config()->get() gets a NULL value from the database, so the third parameter will be returned as the default value.

However, if the site admin sets another server, then the URL of that server will be stored in the database. If afterwards the site admin deletes that custom server again and leaves the input field blank, then DI::config()->get() will get a result from the database -- an empty string. In that case, the empty string will be returned. This is the reason why I'm checking the value with the empty() function.

An alternative approach would be to specifically check in url_replace_addon_admin_post() if the posted string is empty. If it is, I could then use DI::config->delete() instead of DI::config->set(), that should solve the problem.

Thanks for your review. Unfotunately, the third parameter of get() does not work in this case, I've tried that. When the addon is freshly installed, there is no configuration in the database. In that case, `DI::config()->get()` gets a NULL value from the database, so the third parameter will be returned as the default value. However, if the site admin sets another server, then the URL of that server will be stored in the database. If afterwards the site admin deletes that custom server again and leaves the input field blank, then `DI::config()->get()` will get a result from the database -- an empty string. In that case, the empty string will be returned. This is the reason why I'm checking the value with the `empty()` function. An alternative approach would be to specifically check in `url_replace_addon_admin_post()` if the posted string is empty. If it is, I could then use `DI::config->delete()` instead of `DI::config->set()`, that should solve the problem.
toddy commented 2024-03-14 09:42:06 +01:00
Outdated
Review
Copy link

The alternative approach could look like this:

	$server = rtrim(trim($_POST['nitter_server']), '/');
	if (empty($server)) {
		DI::config()->delete('url_replace', 'nitter_server');
	} else {
		DI::config()->set('url_replace', 'nitter_server', $server);
	}

Should I rewrite the addon in that style?

The alternative approach could look like this: ```php $server = rtrim(trim($_POST['nitter_server']), '/'); if (empty($server)) { DI::config()->delete('url_replace', 'nitter_server'); } else { DI::config()->set('url_replace', 'nitter_server', $server); } ``` Should I rewrite the addon in that style?
heluecht commented 2024-03-15 04:44:18 +01:00
Outdated
Review
Copy link

Looks better.

Looks better.
MrPetovan commented 2024-03-15 11:11:42 +01:00
Outdated
Review
Copy link

As a general rule, the default value parameter of both Config->get and IConfig->get is now deprecated in favor of using the short ternary operators ?: and ??. This allows for a better control for default values.

As a general rule, the default value parameter of both `Config->get` and `IConfig->get` is now deprecated in favor of using the short ternary operators `?:` and `??`. This allows for a better control for default values.
toddy commented 2024-03-16 11:43:10 +01:00
Outdated
Review
Copy link

I've just pushed a commit which uses this pattern. Please take a look.

I've just pushed a commit which uses this pattern. Please take a look.
👍 1
$invidious_server = DI::config()->get('url_replace', 'invidious_server');
if (empty($invidious_server)) {
$invidious_server = 'https://yewtu.be';
}
$invidious_server_enabled = DI::config()->get('url_replace', 'invidious_server_enabled', true);
if ($invidious_server_enabled) {
$replacements = array_merge($replacements, [
'https://www.youtube.com' => $invidious_server,
'https://youtube.com' => $invidious_server,
'https://m.youtube.com' => $invidious_server,
'https://youtu.be' => $invidious_server,
]);
}
$proxigram_server = DI::config()->get('url_replace', 'proxigram_server');
if (empty($proxigram_server)) {
$proxigram_server = 'https://proxigram.lunar.icu';
}
// Handle some of twitter and youtube
$replacements = [
'https://mobile.twitter.com' => $nitter_server,
'https://twitter.com' => $nitter_server,
'https://mobile.x.com' => $nitter_server,
'https://x.com' => $nitter_server,
'https://www.youtube.com' => $invidious_server,
'https://youtube.com' => $invidious_server,
'https://m.youtube.com' => $invidious_server,
'https://youtu.be' => $invidious_server,
$proxigram_server_enabled = DI::config()->get('url_replace', 'proxigram_server_enabled', true);
if ($proxigram_server_enabled) {
$replacements = array_merge($replacements, [
'https://www.instagram.com' => $proxigram_server,
'https://instagram.com' => $proxigram_server,
'https://ig.me' => $proxigram_server,
];
]);
}
foreach ($replacements as $server => $replacement) {
if (strpos($b['html'], $server) !== false) {
$b['html'] = str_replace($server, $replacement, $b['html']);