CRUD icon indicating copy to clipboard operation
CRUD copied to clipboard

Add support for pronouns (🍳au 🧀)

Open julienmru opened this issue 4 years ago • 23 comments

In languages other than English (French in my case), pronouns were missing, making some translations grammatically incorrect. Some PR attempted to fix it in an unnatural way, eg. #1790.

This is an attempt to fix it. It may not be perfect for all languages but it should work for German (as far as I remember).

Speaking about German, I first thought of doing a masculine/feminine thing but German has neutral so we need to handle that too. Also, in some cases le or la in French becomes l' (when the noun begins by a vowel). Even English could benefit from it, eg. Add an entry vs Add a page.

If you want to use this code, just add the following in your controller:

CRUD::setEntityPronounStrings('un', 'le', 'des', 'les');

  1. indefinite singular pronoun
  2. definite singular pronoun
  3. indefinite plural pronoun
  4. definite plural pronoun

I believe this should address most cases due to flexibility. If no pronoun is set, no change is visible.

I also added two missing French translations and corrected some typos in French in this PR.

julienmru avatar Aug 08 '20 02:08 julienmru

BOOM! Your first PR with us, thank you so much! Someone will take a look at it shortly.

Please keep in mind that:

  • if this constitutes a breaking change, it might take quite a while for this to get merged; we try to emulate the Laravel release cycle as much as possible, so developers can upgrade both software once; this means a new big release every ~6 months;
  • even if it's a non-breaking change, it might take a few days/weeks for the PR to get merged; unless it's a no-brainer, we like to have some community feedback on new features, before we merge them; this leads to higher-quality code, in the end; we learnt this the hard way :-)
  • not all PRs get merged; sometimes we just have to hold out new features, to keep the packages lean; sometimes we don't include features that only apply to niche use cases;
  • we're not perfect; if you think we're wrong, call us out on it; but in a kind way :-) we all make mistakes, best we learn from them and build better software together;

Thank you!

-- Justin Case The Backpack Robot

welcome[bot] avatar Aug 08 '20 02:08 welcome[bot]

Thank you @julienmru ! Indeed articles&pluralization are things that seems easy enough to fix, but might actually be difficult to achieve in a way that would fit all languages. Thanks for submitting this! Let's see how many languages this solution fits.

@tabacitu - in Romanian it's similar, but not quite. So this would maybe be useful if you could do: CRUD::setEntityPronounStrings('o', null, null, null); But the benefit is very small. It's not gramatically incorrect to without them. So Romanian doesn't need them.

@pxpm - how is it in Portuguese?

@loilo / @marconett - would this be useful for German too?

@eduardoarandah / @MarcosBL - would this be useful in Spanish?

@OliverZiegler / @tswonke - would this be useful in German?

@EmanueleCoppola / @rinodrummer - would this be useful in Italian?

@martijnb92 - would this be useful in Dutch?

@ziming any chance you speak Chinese too? Would this help?

Anybody else, please pitch in with your language and let us know!

tabacitu avatar Aug 17 '20 19:08 tabacitu

It can absolutely be usefull in Dutch, only one change would be needed in src/resources/views/crud/edit.blade.php definite should be used instead of indefinite in the subheading, but that might be different in other languages...

Examples could be: CRUD::setEntityPronounStrings('een', 'de', null, null); CRUD::setEntityPronounStrings('een', 'het', null, null);

martijnb92 avatar Aug 17 '20 20:08 martijnb92

@tabacitu in italian it maybe useful too.

Like for the french example, we have various form of grammar articles and pronouns.

rinodrummer avatar Aug 17 '20 20:08 rinodrummer

So if I understand this correctly, this optionally changes the headlines of the create/update/show/reorder pages to the following if applied to English:

Operation What's shown in current backpack With pronoun
Create  Add Topic Add a Topic
Update Edit Topic Edit the Topic
Show Preview Topic Preview a Topic
Reorder  Reorder Topics Reorder the Topics

In German, this would mean:

Operation What's shown in current backpack With pronoun Actually correct Correct as well
Create  Anlegen: Thema Anlegen: ein Thema Thema anlegen Anlegen: Thema
Update Bearbeiten Thema Bearbeiten das Thema Thema bearbeiten Bearbeiten: Thema
Show Vorschau Thema Vorschau ein Thema Vorschau des Themas Vorschau: Thema
Reorder  Sortiere Themen Sortiere die Themen Themen sortieren Sortieren: Themen

Altough it currently is wrong in most cases anyway, it would be "more wrong" with the pronoun added :)

marconett avatar Aug 18 '20 09:08 marconett

Thanks @marconett

tswonke avatar Aug 18 '20 09:08 tswonke

So if I understand this correctly, this optionally changes the headlines of the create/update/show/reorder pages to the following if applied to English:

Operation What's shown in current backpack With pronoun
Create  Add Topic Add a Topic
Update Edit Topic Edit the Topic
Show Preview Topic Preview a Topic
Reorder  Reorder Topics Reorder the Topics

I would like to specify this thing: in italian, today can even be used the current form (without pronoun); due to the big influence of the english nowadays, it's an accepted form, even if not totally grammatically correct.

rinodrummer avatar Aug 18 '20 09:08 rinodrummer

+1 here; this is a non breaking change with sensible defaults, and could be helpful in Spanish as we have different articles/pronouns and words for almost every singular, plural, masculine and feminine word, sample:

CRUD::setEntityPronounStrings('un', 'el', 'unos', 'los');

MarcosBL avatar Aug 18 '20 09:08 MarcosBL

It can absolutely be usefull in Dutch, only one change would be needed in src/resources/views/crud/edit.blade.php definite should be used instead of indefinite in the subheading, but that might be different in other languages...

Examples could be: CRUD::setEntityPronounStrings('een', 'de', null, null); CRUD::setEntityPronounStrings('een', 'het', null, null);

@martijnb92 was right, also for French. It was my mistake as the singular indefinite pronoun is the same word as "one" so I was meaning "Edit one category", not "Edit a category". "Edit the category" is indeed better. I commited the change.

julienmru avatar Aug 18 '20 10:08 julienmru

The inspection completed: 4 new issues, 7 updated code elements

scrutinizer-notifier avatar Aug 18 '20 10:08 scrutinizer-notifier

In Portugal we have demonstrative pronouns o, a, os, as, "Delete the article" => "Apagar o artigo" but only "Apagar artigo" is okay too.

And also the undefined pronouns um, uma, uns, umas, "Add an Article" => "Adicionar um Artigo" but again, "Adicionar Artigo" is ok.

Best, Pedro

pxpm avatar Aug 22 '20 21:08 pxpm

My 2 cents: I don't think it's efficient to abstract anything related to natural language.

eduardoarandah avatar Aug 24 '20 20:08 eduardoarandah

I'm afraid I can't help much, most of my chinese communication these days are verbal with family. I use English at work for written communication.

But i think chinese pronoun are sort of like english, he is 他 and she is 她

Before western influence, I think there is no concept of gender pronouns in written chinese.

ziming avatar Aug 25 '20 11:08 ziming

@tabacitu There may be another (better?) way to handle it :)

In the different view I edited, instead of the original trans('backpack::crud.add').' '.$this->crud->entity_name;, you could use $this->crud->translate('crud.add', $this->crud->entity_name) where we can override translate to a custom function returning any string (the default function returning trans('backpack::'.message).' '.$entity_name).

Let me know if that makes sense. Imho, it would allow any language variation possible and even handle German as explained by @marconett.

@pxpm, @loilo, @marconett, @eduardoarandah, @MarcosBL, @OliverZiegler, @tswonke, @EmanueleCoppola, @rinodrummer, @martijnb92, @ziming: what do you think?

julienmru avatar Aug 26 '20 06:08 julienmru

@julienmru, maybe $this->crud->trans() could be better because is shorter than $this->crud->translate().

rinodrummer avatar Aug 26 '20 06:08 rinodrummer

If intresting, Russian doesn't need them, but need different entity ending is some case $this->crud->setEntityNameStrings('проект', 'проекты');

Operation What's shown in current backpack Correctly
Create Добавить проект Добавить проект
Update Редактировать проект Редактировать проект
Show Предпросмотр проект Предпросмотр проекта
Reorder Изменить порядок проекты Изменить порядок проектов
Back link (Back to all projects) Вернуться к списку проекты Вернуться к списку проектов

lotarbo avatar Aug 29 '20 17:08 lotarbo

Thank you everyone for pitching in!

I'm back before we're actually working on 4.2.0, in part thanks to @julienmru nudging me to reply and in part because I've moved to a French-speaking region for 3 months now, so... you know... now I speak all this French:

200

Fun fact: this, turns out, is wrong... "omelette au fromage" would be the correct form. Which I think is just a perfect metaphor for what we're trying to here, in this PR 😂 We're trying to go from "omlette du fromage" which is technically wrong, to "omelette au fromage" which would be gramatically correct. The question, though is... is it worth it?! And if so, what's the best way to do it?


Since @julienmru was kind enough to work on this PR, and find solutions to try to accommodate all languages, and everybody's helped with the particularities of their own language, I was initially inclined to say "hell yeah let's do it".

But 😀 Reading your replies, and going through this over and over again... it makes me wonder if it's worth it. Yes we do want Backpack to be as "correct" as possible in as many of the 3,982 written languages as possible. But. Making sure we accommodate to the particularities of even the top 20 languages will probably be a nightmare to maintain 👀 And we have to make sure what we add is better than what was before. Because it looks like in some cases what was before was "ok" but what we'll introduce will be "wrong" (so... worse). The more I read the thread the more I think @eduardoarandah may be right in his statement above:

My 2 cents: I don't think it's efficient to abstract anything related to natural language.


But anyway, let's make some productive steps here. I think two questions need to be asked:

(1) How many languages actually NEED this?

Like... need-need, it's a MUST for them? In what languages is it plainly unacceptable the way it is now? I'll go through the thread and compile a list here, please let me know if I'm wrong, or where I'm missing information and I'll update it. Because as @rinodrummer says here... it's possible that the English influence in UI has just made no-pronouns in UIs acceptable in most other languages.

Language Currently (without pronouns) Proposal (with pronouns)
English Correct Better
Romanian OK Better
Portuguese OK Better
Italian OK Better
French OK Better
Spanish OK?! Better
German OK WORSE
Dutch OK?! Better but needs to be slightly different
Russian OK?! Not needed, but could use different word endings

I might be wrong here, please correct me if it's not "OK" in your language right now. As in... acceptable. But right off the bat, it looks like it's already OK in most languages. So to justify adding this feature, we need it to be really needed by one or more languages (and it doesn't look like it), or to be really easy to maintain. Which brings us to...

(2) Can we fix this problem in a broader way, without tying Backpack code to language specifics, like pronouns, sexes, articles, etc?

If it's something people care about in some language, isn't it already possible in Backpack to modify most operation texts, in the controller?

Looking at all the places where this is needed in French, perhaps a better way to solve this would be to make text like "Back to all products" as easy to overwrite in their entirety, just like you can overwrite the Title, Heading, Subheading. That should make it easier to customize it, without being difficult to maintain. Right?

What do you think @julienmru ? Wouldn't this approach work better, even for French?

PS. Note that this is tagged 4.2.0 - we don't plan to merge this in 4.1.x, that's under a feature freeze.

tabacitu avatar Nov 12 '20 16:11 tabacitu

Hi @tabacitu, @promatik Wouldn't be easier if we publish the translation for the specific language use cases and update them to be easier and more cleaner than writing some new logic and spending time on some small cases?

soufiene-slimi avatar Nov 15 '20 22:11 soufiene-slimi

Wouldn't be easier if we publish the translation for the specific language use cases and update them to be easier and more cleaner than writing some new logic and spending time on some small cases?

@soufiene-slimi not exactly. You can do that, sure, but some languages use different syntaxes depending on the entity gender and stuff. So if you overwrite the language file from "edit" to:

  • English "edit the"
  • French "modifier le"

in English it'll be fine all the time, but in French it won't be ok for all CRUDs:

  • modifier le sac - OK
  • modifier le avion - not ok, should be l'avion
  • modifier le voiture - not ok, should be la voiture

So there's no way to do this by editing the existing language files, afaik.

tabacitu avatar Nov 21 '20 11:11 tabacitu

Hi @tabacitu, what am trying to say is, it's not worth it, you will end up in a closed route, sorry to say that, but it's the truth, I speak french, english and arabic, and I can say that we will end up doing some exceptions for some words, also, as you said, who will maintain this, this will be a real nightmare, Backpack support to override almost everything, I think this is really enough for developers, also, in most cases you will end up changing some crud text based on a client requests.

soufiene-slimi avatar Nov 21 '20 12:11 soufiene-slimi

@tabacitu I had actually proposed earlier another way to handle it:

In the different view I edited, instead of the original trans('backpack::crud.add').' '.$this->crud->entity_name;, you could use $this->crud->translate('crud.add', $this->crud->entity_name) where we can override translate to a custom function returning any string (the default function returning trans('backpack::'.message).' '.$entity_name).

My idea behind it is to keep the various use cases out of Backpack but through the override. That would cancel my PR but solve language issues universally.

My overridden function would look like:

function trans($dish, $flavour) {
     if ($flavour == 'fromage') {
        return $dish . ' au ' . $flavour;
    } else {
        return $dish . ' à la ' . $flavour;
    }
}

$cheese_omelette = trans('omelette', 'fromage');
$cream_omelette = trans('omelette', 'crème');

Let me know if that makes sense.

julienmru avatar Nov 21 '20 12:11 julienmru

@julienmru yup I saw your solution with a function. At first I thought it would be better (definitely), but:

  • it would still have a lot of moving parts (by that I mean it needs to act considerably different for each language);
  • it would still need to be maintained by us since it's in the CRUD;
  • it would be somewhat confusing/false for it to be held by the CRUD, because you might want to overwrite language lines from outside the CRUD too; think dashboard/menu/widgets etc;

But now that we've gone over this again... here's what struck me. 😀 Why would this be limited to being a Backpack function? Why not broaden the scope, and make it a Laravel function? Instead of having $this->crud->trans() why not just overwrite Laravel's trans() function? After all:

  • all Laravel helpers are loaded using if (!function_exists()) so it should be easy to just load a custom trans() function that works differently for particular strings, then falls back to the default Laravel behaviour;
  • it could be used to fix the same problem in other Laravel packages too;
  • it would need ZERO rewriting of existing code;
  • it would need ZERO maintenance for us (it's a tutorial);

Am I missing something here @julienmru ? Wouldn't this work better as a "tutorial" rather than a "feature" itself?

The only downside I see is that that function could become very bloated. And all its logic will be run on every trans() call. But then again, a similar thing would have been true if we made a $this->crud->trans() function, with the impact limited to the admin panel though. Maybe performance is a non-issue though. Since it's just string manipulation and conditionals, it's possible the performance impact will be minimal. After all Laravel does A LOT of these things under the hood anyway, so what's a few more? So yes the function could end up something like:

public function trans($string, $fallback) {
    if ($string == 'backpack::crud.add') {
        return 'smth_else';
    }

    if (Str::startsWith('smth')) {
        return 'bla bla';
    }

    switch ($string) // bla-bla

    if (CRUD::smth()) {
       return 'smth else';
    }

    // return default behaviour;
}

Which could become pretty big if you're not careful. But then again... this would be so so easy to read and modify. Wouldn't it @julienmru ? What do you think - wouldn't overwriting the trans() function be the best solution here, am I missing something?

tabacitu avatar Nov 23 '20 11:11 tabacitu

@tabacitu On another project, I ended up overriding trans indeed :)

However, the overidable trans function with two args like I mentionned in https://github.com/Laravel-Backpack/CRUD/pull/3106#issuecomment-731570994 would still be preferable as some locales are more complex than just adding a word before the pronoun, and even some pronouns in French are not usuable due to the extra space, eg. Modifier l' évènement should be Modifier l'évènement, something we can't achieve right now because the space is hard-coded in Backpack's view.

Also, I can't keep Ajouter in the breadcrumb since the same key is being used:

image

For other interested, here's the kind of code I used (I then specify the gender is the model):

function trans($key = null, $replace = [], $locale = null) {
        if (is_null($key)) {
            return app('translator');
        }

        $trans = app('translator')->get($key, $replace, $locale);


        switch($key) {
        	case 'backpack::crud.add':
        		if (CRUD::getModel()->gender ?? 'masculine' == 'feminine') $trans .= ' une';
        		else $trans .= ' un';
        		break;
        	case 'backpack::crud.edit':
			if (request()->is(config('backpack.base.route_prefix').'/*/edit')) {
	        		if (CRUD::getModel()->gender ?? 'masculine' == 'feminine') $trans .= ' une';
	        		else $trans .= ' un';
	        	}
        		break;
        	case 'backpack::crud.back_to_all':
        		$trans .= ' des';
        		break;
        }

        return $trans;

}

julienmru avatar Jun 08 '21 08:06 julienmru

Hey everyone,

Sorry to be the bearer of bad news. But I've decided not to do this. I think:

  • what we have now covers >80% of the use cases already
  • 15% more are covered by the fact that people have gotten used to English-like translations
  • making a HUGE breaking change, for all languages, does not justify to serve the remaining 5%

Let's KISS here, and stick with what you've got. Thank you everybody for your input, thank you @julienmru for the PR, but I'm going to close this without merging. Please don't let this discourage you. I'm shutting down my own PRs too, regularly, it's just the way things go. If you (or anybody else) think I'm wrong here and it does make sense to merge, that it's important for us to do so - let me know and we'll taken another look, re-open. I'm human, I make mistakes all the time.

Cheers!

tabacitu avatar Oct 11 '23 11:10 tabacitu