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

configurable languages #78

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

GaRaOne
Copy link
Contributor

@GaRaOne GaRaOne commented Jul 15, 2024

the layout and language files are not yet university-independent.

unfortunately we do not have the time to adapt all languages. therefore we have decided on a subset and have made this configurable in the .env.

this also has the advantage that settings.php no longer contains any magic/redundant knowledge about the supported languages.

since php is not really my everyday language, it would be especially good to check if the following check is correct at this point:
if ( !count( $activeLanguages ) ) {

@Ariansdf
Copy link
Collaborator

Hey Steffen,
Thanks for the pull request.
I believe a more "robust" way to do the "if" would be checking it with empty. like:

    // Split the string into an array and remove empty elements
    $activeLanguages = array_filter(explode(' ', $activeLanguagesStr));

    if (empty($activeLanguages)) {
        // Default if no active languages are set
        return ['de_DE', 'en_US', 'es_ES', 'fr_FR', 'it_IT'];
    }

and IMHO I think exploding the array with empty spaces can later cause some troubles. when someone for example forgets to put a space there or writes it with comma and they think that the code doesn't work. why didn't you use commas?

@GaRaOne
Copy link
Contributor Author

GaRaOne commented Aug 1, 2024

i am completely dispassionate as long as the functionality is there. you are welcome to adapt everything so that it corresponds to your ideas and conventions.

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

Successfully merging this pull request may close these issues.

2 participants