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.

Sign up for GitHub

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

Jump to bottom

New language:variables root #6212

Draft
wants to merge 6 commits into
base: v5/develop
Choose a base branch
from
Draft

New language:variables root #6212

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ce59f3
New `translations` root
afbora Jan 26, 2024
437745f
Apply code reviews
afbora Jan 27, 2024
b1b372f
Improvements
afbora Sep 13, 2024
b171903
Rename root with `language:variables`
afbora Sep 16, 2024
1628be7
Swtich from `::translations()` to `::variables()`
afbora Sep 16, 2024
b56dad6
Apply suggestions from code review
afbora Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions config/areas/languages/views.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
$language = Find::language($code);
$link = '/languages/' . $language->code();
$strings = [];
$foundation = $kirby->defaultLanguage()->translations();
$translations = $language->translations();
$foundation = $kirby->defaultLanguage()->variables()->toArray();
$variables = $language->variables()->toArray();

// TODO: update following line and adapt for update and
// delete options when `languageVariables.*` permissions available
Expand All @@ -29,7 +29,7 @@
foreach ($foundation as $key => $value) {
$strings[] = [
'key' => $key,
'value' => $translations[$key] ?? null,
'value' => $variables[$key] ?? null,
'options' => [
[
'click' => 'update',
Expand Down
18 changes: 12 additions & 6 deletions src/Cms/AppTranslations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ protected function i18n(): void
$this->multilang() === true &&
$language = $this->languages()->find($locale)
) {
$data = [...$data, ...$language->translations()];
$data = [
...$data,
...$language->variables()->toArray()
];
}


Expand Down Expand Up @@ -135,7 +138,10 @@ public function translation(string|null $locale = null): Translation

// inject current language translations
if ($language = $this->language($locale)) {
$inject = [...$inject, ...$language->translations()];
$inject = [
...$inject,
...$language->variables()->toArray()
];
}

// load from disk instead
Expand All @@ -160,14 +166,14 @@ public function translations(): Translations
// injects languages translations
if ($languages = $this->languages()) {
foreach ($languages as $language) {
$languageCode = $language->code();
$languageTranslations = $language->translations();
$languageCode = $language->code();
$languageVariables = $language->variables()->toArray();

// merges language translations with extensions translations
if (empty($languageTranslations) === false) {
if (empty($languageVariables) === false) {
$translations[$languageCode] = [
...$translations[$languageCode] ?? [],
...$languageTranslations
...$languageVariables
];
}
}
Expand Down
54 changes: 27 additions & 27 deletions src/Cms/Core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,33 +331,33 @@ public function load(): Loader
public function roots(): array
{
return $this->cache['roots'] ??= [
'kirby' => fn (array $roots) => dirname(__DIR__, 2),
'i18n' => fn (array $roots) => $roots['kirby'] . '/i18n',
'i18n:translations' => fn (array $roots) => $roots['i18n'] . '/translations',
'i18n:rules' => fn (array $roots) => $roots['i18n'] . '/rules',

'index' => fn (array $roots) => static::$indexRoot ?? dirname(__DIR__, 3),
'assets' => fn (array $roots) => $roots['index'] . '/assets',
'content' => fn (array $roots) => $roots['index'] . '/content',
'media' => fn (array $roots) => $roots['index'] . '/media',
'panel' => fn (array $roots) => $roots['kirby'] . '/panel',
'site' => fn (array $roots) => $roots['index'] . '/site',
'accounts' => fn (array $roots) => $roots['site'] . '/accounts',
'blueprints' => fn (array $roots) => $roots['site'] . '/blueprints',
'cache' => fn (array $roots) => $roots['site'] . '/cache',
'collections' => fn (array $roots) => $roots['site'] . '/collections',
'commands' => fn (array $roots) => $roots['site'] . '/commands',
'config' => fn (array $roots) => $roots['site'] . '/config',
'controllers' => fn (array $roots) => $roots['site'] . '/controllers',
'languages' => fn (array $roots) => $roots['site'] . '/languages',
'license' => fn (array $roots) => $roots['config'] . '/.license',
'logs' => fn (array $roots) => $roots['site'] . '/logs',
'models' => fn (array $roots) => $roots['site'] . '/models',
'plugins' => fn (array $roots) => $roots['site'] . '/plugins',
'sessions' => fn (array $roots) => $roots['site'] . '/sessions',
'snippets' => fn (array $roots) => $roots['site'] . '/snippets',
'templates' => fn (array $roots) => $roots['site'] . '/templates',
'roles' => fn (array $roots) => $roots['blueprints'] . '/users',
'kirby' => fn(array $roots) => dirname(__DIR__, 2),
'i18n' => fn(array $roots) => $roots['kirby'] . '/i18n',
'i18n:translations' => fn(array $roots) => $roots['i18n'] . '/translations',
'i18n:rules' => fn(array $roots) => $roots['i18n'] . '/rules',
'index' => fn(array $roots) => static::$indexRoot ?? dirname(__DIR__, 3),
'assets' => fn(array $roots) => $roots['index'] . '/assets',
'content' => fn(array $roots) => $roots['index'] . '/content',
'media' => fn(array $roots) => $roots['index'] . '/media',
'panel' => fn(array $roots) => $roots['kirby'] . '/panel',
'site' => fn(array $roots) => $roots['index'] . '/site',
'accounts' => fn(array $roots) => $roots['site'] . '/accounts',
'blueprints' => fn(array $roots) => $roots['site'] . '/blueprints',
'cache' => fn(array $roots) => $roots['site'] . '/cache',
'collections' => fn(array $roots) => $roots['site'] . '/collections',
'commands' => fn(array $roots) => $roots['site'] . '/commands',
'config' => fn(array $roots) => $roots['site'] . '/config',
'controllers' => fn(array $roots) => $roots['site'] . '/controllers',
'language:variables' => null,
'languages' => fn(array $roots) => $roots['site'] . '/languages',
'license' => fn(array $roots) => $roots['config'] . '/.license',
'logs' => fn(array $roots) => $roots['site'] . '/logs',
'models' => fn(array $roots) => $roots['site'] . '/models',
'plugins' => fn(array $roots) => $roots['site'] . '/plugins',
'roles' => fn(array $roots) => $roots['blueprints'] . '/users',
'sessions' => fn(array $roots) => $roots['site'] . '/sessions',
'snippets' => fn(array $roots) => $roots['site'] . '/snippets',
'templates' => fn(array $roots) => $roots['site'] . '/templates',
];
}

Expand Down
40 changes: 32 additions & 8 deletions src/Cms/Language.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class Language implements Stringable
protected bool $single;
protected array $slugs;
protected array $smartypants;
protected array $translations;
protected string|null $url;
protected LanguageVariables $variables;

/**
* Creates a new language object
Expand All @@ -68,14 +68,15 @@ public function __construct(array $props)
$this->single = $props['single'] ?? false;
$this->slugs = $props['slugs'] ?? [];
$this->smartypants = $props['smartypants'] ?? [];
$this->translations = $props['translations'] ?? [];
$this->url = $props['url'] ?? null;

if ($locale = $props['locale'] ?? null) {
$this->locale = Locale::normalize($locale);
} else {
$this->locale = [LC_ALL => $this->code];
}

$this->variables = new LanguageVariables($this, $props['translations'] ?? []);
afbora marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -130,7 +131,7 @@ public function clone(array $props = []): static
'name' => $this->name,
'slugs' => $this->slugs,
'smartypants' => $this->smartypants,
'translations' => $this->translations,
'translations' => $this->variables->toArray(),
afbora marked this conversation as resolved.
Show resolved Hide resolved
'url' => $this->url,
], $props));
}
Expand Down Expand Up @@ -222,6 +223,7 @@ public function delete(): bool
LanguageRules::delete($this);

// apply before hook
/** @var \Kirby\Cms\Language $language */
$language = $kirby->apply(
'language.delete:before',
['language' => $this],
Expand All @@ -235,6 +237,9 @@ public function delete(): bool
throw new Exception('The language could not be deleted');
}

// delete custom translations file if defined
$language->variables()->delete();

// if needed, convert content storage to single lang
foreach ($kirby->models() as $model) {
if ($language->isLast() === true) {
Expand Down Expand Up @@ -482,12 +487,19 @@ public function save(): static
'direction' => $this->direction(),
'locale' => Locale::export($this->locale()),
'name' => $this->name(),
'translations' => $this->translations(),
'translations' => $this->variables()->toArray(),
afbora marked this conversation as resolved.
Show resolved Hide resolved
'url' => $this->url,
];

ksort($data);

// save translations to the custom root and remove translations
// to prevent duplication write into the language file
if ($this->variables()->root() !== null) {
$this->variables()->save($data['translations'] ?? []);
$data['translations'] = [];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead of unsetting it here, we do an if-else clause: The if part writes it to the dedicated root, the else part adds it to the $data array?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If custom root is already set (root !== null), it empties the data.translations array and writes to the custom root. If custom root is not set, it saves the data array normally as before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant:

		$data = [
			...$existingData,
			'code'         => $this->code(),
			'default'      => $this->isDefault(),
			'direction'    => $this->direction(),
			'locale'       => Locale::export($this->locale()),
			'name'         => $this->name(),
			'url'          => $this->url,
		];

		ksort($data);

		// save translations to the custom root and remove translations
		// to prevent duplication write into the language file
		if ($this->translations()->root() !== null) {
			$this->translations()->save();
		} else {
			$data['translations'] = $this->translations()->toArray();
		}

(not passing anything to $this->translations()->save() here based on the other suggestion I make below)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to empty to $data['translations'] index if custom root defined. Because after few lines later will run Data::write() and same translation strings to be written to language translations index. $data['translations'] = [] prevents it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we don't add it to $data in the first place, we wouldn't need to unset it, right? Or could $existingData also have a translations key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, $existingData mean /site/languages/*.php data and currently languages have translations key.


Data::write($this->root(), $data);

return $this;
Expand Down Expand Up @@ -548,11 +560,14 @@ public function toArray(): array
}

/**
* Returns the translation strings for this language
* Alias for Language::variables()
*
* @deprecated 5.0.0 Use `::variables()` instead
* @todo 7.0.0 Remove the method
*/
public function translations(): array
public function translations(): LanguageVariables
{
return $this->translations;
return $this->variables();
}

/**
Expand Down Expand Up @@ -581,6 +596,7 @@ public function update(array|null $props = null): static


// trigger before hook
/** @var \Kirby\Cms\Language $language */
$language = $kirby->apply(
'language.update:before',
[
Expand All @@ -594,7 +610,7 @@ public function update(array|null $props = null): static
$language = $language->clone($props);

if (isset($props['translations']) === true) {
$language->translations = $props['translations'];
$language->variables = $language->variables->update($props['translations'] ?? null);
}

// validate the language rules after before hook was applied
Expand Down Expand Up @@ -644,4 +660,12 @@ public function variable(string $key, bool $decode = false): LanguageVariable

return new LanguageVariable($this, $key);
}

/**
* Returns the language variables object for this language
*/
public function variables(): LanguageVariables
{
return $this->variables;
}
}
32 changes: 15 additions & 17 deletions src/Cms/LanguageVariable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ public static function create(
throw new InvalidArgumentException('The variable needs a valid key');
}

$kirby = App::instance();
$language = $kirby->defaultLanguage();
$translations = $language->translations();
$kirby = App::instance();
$language = $kirby->defaultLanguage();
$variables = $language->variables()->toArray();

if ($kirby->translation()->get($key) !== null) {
if (isset($translations[$key]) === true) {
if (isset($variables[$key]) === true) {
throw new DuplicateException('The variable already exists');
}

throw new DuplicateException('The variable is part of the core translation and cannot be overwritten');
}

$translations[$key] = $value ?? '';
$variables[$key] = $value ?? '';

$language->update(['translations' => $translations]);
$language->update(['variables' => $variables]);

return $language->variable($key);
}
Expand All @@ -72,11 +72,9 @@ public function delete(): bool
{
// go through all languages and remove the variable
foreach ($this->kirby->languages() as $language) {
$variables = $language->translations();

unset($variables[$this->key]);

$language->update(['translations' => $variables]);
$variables = $language->variables();
$variables->remove($this->key);
$language->update(['variables' => $variables->toArray()]);
}

return true;
Expand All @@ -88,7 +86,7 @@ public function delete(): bool
public function exists(): bool
{
$language = $this->kirby->defaultLanguage();
return isset($language->translations()[$this->key]) === true;
return $language->variables()->get($this->key) !== null;
}

/**
Expand All @@ -104,19 +102,19 @@ public function key(): string
*/
public function update(string|null $value = null): static
{
$translations = $this->language->translations();
$translations[$this->key] = $value ?? '';
$variables = $this->language->variables();
$variables->set($this->key, $value);

$language = $this->language->update(['translations' => $translations]);
$language = $this->language->update(['variables' => $variables->toArray()]);

return $language->variable($this->key);
}

/**
* Returns the value if the variable has been translated.
* Returns the value if the variable has been translated
*/
public function value(): string|null
{
return $this->language->translations()[$this->key] ?? null;
return $this->language->variables()->get($this->key);
}
}
Loading
Loading