About how to optimize and improve translation of commands

[Evgeniy]

2) workbench names use "Workbench"

I would like to try to apply this rule to almost all workbenches step by step. If I have enough strength.
if it doesn't hurt the project, it should remove 800 duplicates.

Pull request is merged and we are waiting for rebuild Crowdin project with this https://github.com/FreeCAD/FreeCAD/pull/5037
for fix this: https://forum.freecadweb.org/viewtopic. ... 84#p535779

I apologize for being in a hurry to solve this problem.

[Evgeniy]

We just need to agree (create a rule and follow it) on which key context name we will use with sGroup values of commands and insert this context in all Command.cpp an Commands.py files in all places where is sGroup value exists.

It's all correct what you write but there is one little issue: it won't work. At least not the way how it is now.

The method getGroupName() returns the member variable sGroup of a command and this method is used in several dialogs like this:

Code: Select all

QString text = qApp->translate(cmd->className(), cmd->getGroupName());

This means the context string is the Command class name itself and when you change the context string inside the class to "Workbench" then qApp->translate() will fail to find the translation. At the moment it works because the .ts files still contain the different contexts of the group name. But as soon as the .ts file are cleaned-up it will stop working.

I have read your proposal several times and I think I understand what you mean.
Could you show links to using these strings:

Code: Select all

QString text = qApp->translate(cmd->className(), cmd->getGroupName());

in source code? I not find examples...
I tried to find examples by searching on github, but I didn't find them... Sorry but I don't know source code fully.
getGroupName() is used but not with combination of cmd->className().

I think your proposal will require, a change getGroupName method:

from https://github.com/FreeCAD/FreeCAD/blob ... and.h#L569

Code: Select all

const char* getGroupName() const { return sGroup; }

to something like this:

Code: Select all

const char* getGroupName() const{
    return QT_TRANSLATE_NOOP("Workbench",sGroup);
}

This needs to weigh everything well...
Are you sure this will work correctly? You ready to update this method (or maybe other methods too)?

What other developers thing about this proposal?

[wmayer]

I think your proposal will require, a change getGroupName method:

Currently it's used in following methods:

• DlgCustomCommandsImp::DlgCustomCommandsImp
• DlgCustomCommandsImp::changeEvent
• CommandModel::data
• DlgCustomKeyboardImp::DlgCustomKeyboardImp
• DlgCustomKeyboardImp::changeEvent
• DlgCustomToolbars::DlgCustomToolbars
• DlgCustomToolbars::changeEvent

I think your proposal will require, a change getGroupName method:

Code: Select all

const char* getGroupName() const{
    return QT_TRANSLATE_NOOP("Workbench",sGroup);
}

No, this is useless. What should lupdate collect when QT_TRANSLATE_NOOP("Workbench",sGroup) is used? It can only collect a string that is directly passed to QT_TRANSLATE_NOOP, e.g. QT_TRANSLATE_NOOP("Workbench","Sketcher")

[Evgeniy]

I analyzed the youre porposal further:

Code: Select all

QString text = qApp->translate(cmd->className(), cmd->getGroupName());

the class to "Workbench" then qApp->translate() will fail to find the translation.

IMO for example that equals:

Code: Select all

QString text = qApp->translate("CmdNumberUnkonow", "Скетч");

because the second value has already been translated in sGroup varible then qApp->translate not find translation and return last argument:

Code: Select all

 "Скетч"

everything is should be work normal.

[Evgeniy]

For example we have:

It's sufficient to simply write

Code: Select all

sGroup = "Sketcher";

and we have this code:

https://github.com/FreeCAD/FreeCAD/blob ... mp.cpp#L96

Code: Select all

QString text = qApp->translate(it->second->className(), it->second->getGroupName());

code turns to:

QString text = qApp->translate("CmdCommandName", "Sketcher");

We will not receive any translation at result...
Sorry, I don't understand the essence of your proposal...

[wmayer]

because the second value has already been translated in sGroup varible then qApp->translate not find translation and return last argument:

No, the second value (that is returned by cmd->getGroupName()) is not translated at this point. The macros QT_TRANSLATE_NOOP or QT_TR_NOOP are only a marker for Qt's lupdate tool to add the strings to the .ts files. For FreeCAD itself they don't have any function.

This is how the macros are defined in the Qt code (see qglobal.h)

Code: Select all

#define QT_TR_NOOP(x) x
#define QT_TR_NOOP_UTF8(x) x
#define QT_TRANSLATE_NOOP(scope, x) x
#define QT_TRANSLATE_NOOP_UTF8(scope, x) x
#define QT_TRANSLATE_NOOP3(scope, x, comment) {x, comment}
#define QT_TRANSLATE_NOOP3_UTF8(scope, x, comment) {x, comment}

The actual translation then happens inside qApp->translate(). The function searches for a translated string of the passed context and untranslated string.

IMO for example that equals:

Code: Select all

QString text = qApp->translate("CmdNumberUnkonow", "Скетч");

No, it's not. And why should an already translated text be passed to qApp->translate? That doesn't make any sense.

[Evgeniy]

Ok. I understand...

QT_TRANSLATE_NOOP not do translation.

Then how to get rid of duplicates, if many method gets translation by using the class name?

Code: Select all

QString text = qApp->translate(cmd->className(), cmd->getGroupName());

I think there are few such functions. What if replace it to:

Code: Select all

QString text = qApp->translate("Workbench", it->second->getGroupName());
// If no result. Try to get translate by old method
if (text==it->second->getGroupName()) text = qApp->translate(it->second->className(), it->second->getGroupName());

Or this bad idea?

[wmayer]

Then how to get rid of duplicates, if many method gets translation by using the class name?

Like I said in my first post:

• In all command classes replace

  Code: Select all

  sGroup = QT_TR_NOOP("Something");

  with

  Code: Select all

  sGroup = "Something";

  i.e. you effectively remove the QT_TR_NOOP
• Then e.g. in the Workbench.cpp files add this:

  Code: Select all

  #if 0 // needed for Qt's lupdate utility
      qApp->translate("Workbench", "Something");
  #endif
  

• Now go through all functions where getGroupName() is used for translation and replace

  Code: Select all

  QString text = qApp->translate(cmd->className(), cmd->getGroupName());
  

  with

  Code: Select all

  QString text = qApp->translate("Workbench", cmd->getGroupName());
  

Important note:
You shouldn't write

Code: Select all

sGroup = QT_TRANSLATE_NOOP("Workbench","Something");

in the command classes because as soon as we want to change the context string it needs to be done again for hundreds of classes.

[wmayer]

I think there are few such functions. What if replace it to:

Code: Select all

QString text = qApp->translate("Workbench", it->second->getGroupName());
// If no result. Try to get translate by old method
if (text==it->second->getGroupName()) text = qApp->translate(it->second->className(), it->second->getGroupName());

Or this bad idea?

As long as the move is not fully performed (i.e. the translations on Crowdin are not done) it is an acceptable workaround. But before doing this in all affected locations it's probably better to add a method to the Command class that does it:

Code: Select all

QString Command::translatedGroupName() const
{
    QString text = qApp->translate("Workbench", getGroupName());
    if (text == QString::fromLatin(getGroupName()))
        text = qApp->translate(className(), getGroupName());
    return text;
}

Btw, IMO the context "Workbench" doesn't fit so well and we better should use "CommandGroup" instead.

[wmayer]

Here is the convenience function Command::translatedGroupName(): git commit bc57ba6027