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

add German informal language #1159

Merged
merged 2 commits into from Dec 12, 2018

Conversation

2 participants
@ezzra
Copy link

ezzra commented Dec 3, 2018

This commit adds German informal language ("Du" instead of "Sie") based on the current formal language files. Fixes this part of #890.

@ssddanbrown

This comment has been minimized.

Copy link
Member

ssddanbrown commented Dec 10, 2018

Thanks for putting this pull request together @ezzra.

To be honest, I'm still a little apprehensive to merge this in. So far I only really have a sample size of two voices in regards to the benefit of supporting both language sets. Taking a quick look at wordpress and gitlab, two big OS projects fair-sized teams, Not even they appear to directly support this.

Is it only a subset of translations that are different or quite a lot? If so it might be better to set up mechanisms so only the differences are defined, then the systems fallsback to DE before falling back to EN as a last resort. That way there'd be much less to maintain.

@ezzra

This comment has been minimized.

Copy link
Author

ezzra commented Dec 10, 2018

Not even they appear to directly support this.

but both of them use the informal "Du" and not the formal "Sie" like here ;-) But even wordpress introduced formal "Sie" language variant finally some month before. Its actually an issue, in coding communities for example its very much common to use informal "Du", no one would use "Sie". That is why I think Gitlab will not introduce the formal language. But in Wordpress the users are not part of any community, so its quite right to offer also the formal language variant.

However, you could rather consider not offering the formal language, but not offering the informal language wouldn't fit to this kind of project. But you are completely right, it would be much better if one would be based on the other, but it looks like there is only one fallback in laravel translation system?!

@ezzra

This comment has been minimized.

Copy link
Author

ezzra commented Dec 10, 2018

but it looks like there is only one fallback in laravel translation system?!

ok I set up working mechanism like:

<?php
$de_formal = (include resource_path() . '/lang/de/' . basename(__FILE__));

$de_informal = [
    /**
     * Image Manager
     */
    'image_delete_confirm' => 'Bitte klicke erneut auf löschen, wenn Du dieses Bild wirklich entfernen möchtest.',
    'image_dropzone' => 'Ziehe Bilder hierher oder klicke hier, um ein Bild auszuwählen',
    /**
     * Code editor
     */
];

return array_replace($de_formal, $de_informal);

I will push this later today.

@ezzra ezzra force-pushed the ezzra:german_informal branch from b6f8111 to b55afd2 Dec 11, 2018

@ezzra

This comment has been minimized.

Copy link
Author

ezzra commented Dec 11, 2018

I just force-pushed the new variant with dependence on the formal german language files. I still dont know how to care for the comments within the files where no more language settings exists, in my opinion they could be removed. And maybe there should be a comment in each file, that this is depending on lang/de

How do you think about this new approach @ssddanbrown ?

@ezzra ezzra force-pushed the ezzra:german_informal branch from b55afd2 to a2acd06 Dec 11, 2018

@ssddanbrown

This comment has been minimized.

Copy link
Member

ssddanbrown commented Dec 11, 2018

@ezzra Looks good. Yeah, I'd probably remove any comments they don't have any translations under them.

Looking at the core laravel Illuminate\Translation\Translator class I think it'd be fairly easy to extend to add 'baseLanguage' support. Would also need to setup a custom provider to implement it but simple extend the existing one. This would be a nice step to remove any logic from these translation files and align with how the existing fallback works.

I'd be happy to set this up after merge though.

@ezzra

This comment has been minimized.

Copy link
Author

ezzra commented Dec 12, 2018

I would definitely prefer that, even if this here an ok solution, it's not really nice that it doesn't get any simpler. I was also surprised that the language system doesn't offer that option.

I removed the comments, but it's also not really nice that there is apparently no uniform way in the language files how the comments should be formatted. Some are just // pages others are

/**
* Password Reset
*/`

etc.

@ssddanbrown ssddanbrown added this to the BookStack Beta v0.25.0 milestone Dec 12, 2018

@ssddanbrown ssddanbrown merged commit f943f0d into BookStackApp:master Dec 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

ssddanbrown added a commit that referenced this pull request Dec 12, 2018

Extended translations system for arrays & extension
Extended the base Laravel translation system to
allow a locale to be based upon another.

Also adds functionality to take base & fallback locales into account when fetching
an array of translations.

Related to work done in #1159
@ssddanbrown

This comment has been minimized.

Copy link
Member

ssddanbrown commented Dec 12, 2018

Now merged. As said above, I tweaked the implementation and added tests to cover in 323bff7.

Yeah, Would be good to standardise the comments at some point. Am looking to update the translation helper script at some point to match en line numbers and comments so it will be standardised when I get around to that.

Thanks again for opening this pull request @ezzra

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