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

[Discussion] Add comment to Command, ToolTip. #167

Closed
Kuzma30 opened this issue Jun 7, 2022 · 11 comments · Fixed by FreeCAD/FreeCAD#9440
Closed

[Discussion] Add comment to Command, ToolTip. #167

Kuzma30 opened this issue Jun 7, 2022 · 11 comments · Fixed by FreeCAD/FreeCAD#9440

Comments

@Kuzma30
Copy link

Kuzma30 commented Jun 7, 2022

I suggest adding a comment for tool-tip and command names to facilitate translation. (Since it is sometimes difficult to determine this command name or its tool-tip (description) from the text)
Example.
Change

ui->ManagePreferencePacks->setToolTip(tr("Manage preference packs"));

to

ui->ManagePreferencePacks->setToolTip(tr("Manage preference packs","ToolTip"));

This text will be

    <message>
        <location filename="DlgGeneralImp.cpp" line="91"/>
        <source>Manage preference packs</source>
        <comment>ToolTip</comment>
        <translation type="unfinished"></translation>
    </message>

in the .ts file.
This can be done after 0.20 release.

@Kuzma30
Copy link
Author

Kuzma30 commented Jun 10, 2022

@chennes @luzpaz
I start working for changing after 0.20 release
Now I need help of native English man.
Please look this file (ignore //~ TechDraw===================== string)
Look only fist tooltip change.
Kuzma30/FreeCAD@dee9f26
Is it correct that menu command and tooltip (description of this command) use same text string?
Is correct changing
sToolTipText = QT_TR_NOOP("Insert Dimension");
to
sToolTipText = QT_TR_NOOP("Insert a Dimension");?

PS.
In Ukrainian if menutext and tooltip is the same string translation looks very bad.
I want have ability translate menu text and tooltip separately.

@chennes
Copy link
Member

chennes commented Jun 10, 2022

Very interesting, I never thought of that! In English it is sometimes acceptable to add the article, but I'm sure there are cases where it is not. I suggest that a more robust fix is to add a disambiguation string, so that even in cases where the English is the same for both, other languages are not tied to that.

@Kuzma30
Copy link
Author

Kuzma30 commented Jun 10, 2022

Very interesting, I never thought of that! In English it is sometimes acceptable to add the article, but I'm sure there are cases where it is not. I suggest that a more robust fix is to add a disambiguation string, so that even in cases where the English is the same for both, other languages are not tied to that.

QT_TR_NOOP don't accept disambiguation string:(
QT_TRANSLATE_NOOP3 and QT_TRANSLATE_N_NOOP3 only accept it.

@chennes
Copy link
Member

chennes commented Jun 10, 2022

Those were added in Qt 5.12 I think, so maybe in 0.21 that's a reason to bump our required Qt version.

@chennes
Copy link
Member

chennes commented Jun 10, 2022

No, I'm wrong, QT_TRANSLATE_NOOP3 has been with us for some time, it's fair game right now.

@Kuzma30
Copy link
Author

Kuzma30 commented Jun 11, 2022

[ 95%] Built target FemGui
In file included from /usr/include/x86_64-linux-gnu/qt5/QtGui/qtguiglobal.h:43,
                 from /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qtwidgetsglobal.h:43,
                 from /usr/include/x86_64-linux-gnu/qt5/QtWidgets/qapplication.h:43,
                 from /usr/include/x86_64-linux-gnu/qt5/QtWidgets/QApplication:1,
                 from /home/aleksandr/test_crowdin/FreeCAD/src/Mod/TechDraw/Gui/CommandCreateDims.cpp:25:
/home/aleksandr/test_crowdin/FreeCAD/src/Mod/TechDraw/Gui/CommandCreateDims.cpp: In constructor ‘CmdTechDrawDimension::CmdTechDrawDimension()’:
/home/aleksandr/test_crowdin/FreeCAD/src/Mod/TechDraw/Gui/CommandCreateDims.cpp:128:23: error: cannot convert ‘<brace-enclosed initializer list>’ to ‘const char*’ in assignment
  128 |     sToolTipText    = QT_TRANSLATE_NOOP3("TechDraw","Insert Dimension","ToolTip");
      |                       ^~~~~~~~~~~~~~~~~~

@Kuzma30
Copy link
Author

Kuzma30 commented Jun 11, 2022

@chennes I play with different type of comments
For example

    //: MenuText
    sMenuText       = QT_TR_NOOP("Insert Dimension");
    //: ToolTip
    sToolTipText    = QT_TR_NOOP("Insert Dimension");

produce

    <message>
        <location filename="CommandCreateDims.cpp" line="128"/>
        <location filename="CommandCreateDims.cpp" line="130"/>
        <source>Insert Dimension</source>
        <extracomment>MenuText
----------
ToolTip</extracomment>
        <translation type="unfinished"></translation>
    </message>

I have only one idea (Use of QT_TRANSLATE_NOOP3 need big refactor). Change upper-case letter in tooltip
for example sToolTipText = QT_TR_NOOP("Insert dimension");

Kuzma30/FreeCAD@3f57e92

@chennes
Copy link
Member

chennes commented Jun 11, 2022

The time is right for a big refactor, IMO: we're going to tag the 0.20 release tomorrow night, and then we can start making some of the big changes we've talked about.

@chennes
Copy link
Member

chennes commented Apr 29, 2023

I'm bringing this up again now in advance of the 0.21 feature freeze: the big problem with switching to QT_TRANSLATE_NOOP3 is that it doesn't just give a const char * back, it gives an anonymous struct with two components: the string we want, and the disambiguation string (which is useless to the code). So it's sort of difficult to use the way we want to. We'll want to code a small wrapper for it that extracts just the string we need.

@chennes
Copy link
Member

chennes commented Apr 29, 2023

Bah, it gets pretty ugly. I'm back to your original solution! I'm going to ask for some help on the forums to generate a list of the command where this is a problem. Maybe we can even code this into the cMake file to force developers to do it 😄 .

@chennes
Copy link
Member

chennes commented Apr 29, 2023

Thanks to the regex wizards on the forums, we have the following list of commands whose tooltips should be changed so they don't match their commands:

./src/Gui/CommandLink.cpp:874:        sMenuText     = QT_TR_NOOP("Link actions");
./src/Gui/CommandFeat.cpp:78:    sMenuText     = QT_TR_NOOP("Random color");
./src/Gui/CommandView.cpp:2107:        sMenuText     = QT_TR_NOOP("Toggle axis cross");
./src/Gui/CommandView.cpp:2430:    sMenuText     = QT_TR_NOOP("Zoom In");
./src/Gui/CommandView.cpp:2459:    sMenuText     = QT_TR_NOOP("Zoom Out");
./src/Gui/CommandView.cpp:2625:    sMenuText     = QT_TR_NOOP("Box zoom");
./src/Gui/CommandView.cpp:2695:    sMenuText     = QT_TR_NOOP("Box selection");
./src/Gui/CommandView.cpp:2958:    sMenuText     = QT_TR_NOOP("Box element selection");
./src/Gui/CommandView.cpp:3125:    sMenuText     = QT_TR_NOOP("Measure distance");
./src/Gui/CommandView.cpp:3304:    sMenuText     = QT_TR_NOOP("Clear measurement");
./src/Gui/CommandView.cpp:3334:    sMenuText     = QT_TR_NOOP("Toggle measurement");
./src/Gui/CommandTest.cpp:297:    sMenuText       = "Test functions";
./src/Gui/CommandTest.cpp:717:    sMenuText   = QT_TR_NOOP("Test console output");
./src/Gui/CommandMacro.cpp:240:    sMenuText     = QT_TR_NOOP("Step over");
./src/Gui/CommandMacro.cpp:267:    sMenuText     = QT_TR_NOOP("Step into");
./src/Gui/CommandMacro.cpp:294:    sMenuText     = QT_TR_NOOP("Toggle breakpoint");
./src/Gui/CommandDoc.cpp:1690:        sMenuText     = QT_TR_NOOP("Expression actions");
./src/Gui/CommandWindow.cpp:417:    sMenuText     = QT_TR_NOOP("Activates this window");
./src/Mod/Drawing/Gui/Command.cpp:92:    sMenuText       = QT_TR_NOOP("Insert new drawing");
./src/Mod/Drawing/Gui/Command.cpp:270:    sMenuText       = QT_TR_NOOP("Insert new A3 landscape drawing");
./src/Mod/Fem/Gui/Command.cpp:1139:    sMenuText = QT_TR_NOOP("Electromagnetic constraints");
./src/Mod/Web/Gui/Command.cpp:131:    sMenuText       = QT_TR_NOOP("Refresh web page");
./src/Mod/Web/Gui/Command.cpp:159:    sMenuText       = QT_TR_NOOP("Stop loading");
./src/Mod/Web/Gui/Command.cpp:189:    sMenuText       = QT_TR_NOOP("Zoom in");
./src/Mod/Web/Gui/Command.cpp:218:    sMenuText       = QT_TR_NOOP("Zoom out");
./src/Mod/Web/Gui/Command.cpp:247:    sMenuText       = QT_TR_NOOP("Set URL");
./src/Mod/TechDraw/Gui/CommandDecorate.cpp:290:    sMenuText       = QT_TR_NOOP("Turn View Frames On/Off");
./src/Mod/Sketcher/Gui/CommandSketcherTools.cpp:283:    sMenuText = QT_TR_NOOP("Select redundant constraints");
./src/Mod/Sketcher/Gui/CommandSketcherTools.cpp:345:    sMenuText = QT_TR_NOOP("Select malformed constraints");
./src/Mod/Sketcher/Gui/CommandSketcherTools.cpp:404:    sMenuText = QT_TR_NOOP("Select partially redundant constraints");
./src/Mod/Sketcher/Gui/CommandSketcherTools.cpp:464:    sMenuText = QT_TR_NOOP("Select conflicting constraints");
./src/Mod/Robot/Gui/Command.cpp:52:    sMenuText       = QT_TR_NOOP("Set the home position");
./src/Mod/Robot/Gui/Command.cpp:101:    sMenuText       = QT_TR_NOOP("Move to home");
./src/Mod/Sandbox/Gui/Command.cpp:926:    sMenuText       = "Grab widget";
./src/Mod/Sandbox/Gui/Command.cpp:1047:    sMenuText       = "SoImage node";
./src/Mod/Sandbox/Gui/Command.cpp:1111:    sMenuText       = "GDI widget";
./src/Mod/Sandbox/Gui/Command.cpp:1137:    sMenuText       = "Redirect paint";
./src/Mod/Sandbox/Gui/Command.cpp:1166:    sMenuText       = "Cryptographic Hash";
./src/Mod/Sandbox/Gui/Command.cpp:1188:    sMenuText       = "Widget shape";
./src/Mod/Sandbox/Gui/Command.cpp:1212:    sMenuText     = QT_TR_NOOP("Menger sponge");
./src/Mod/Sandbox/Gui/Command.cpp:1409:    sMenuText       = "Task box";
./src/Mod/PartDesign/Gui/CommandPrimitive.cpp:69:    sMenuText       = QT_TR_NOOP("Create an additive primitive");
./src/Mod/PartDesign/Gui/CommandPrimitive.cpp:244:    sMenuText       = QT_TR_NOOP("Create a subtractive primitive");
./src/Mod/PartDesign/Gui/Command.cpp:345:    sMenuText       = QT_TR_NOOP("Create a sub-object(s) shape binder");
./src/Mod/Part/Gui/Command.cpp:2394:    sMenuText     = QT_TR_NOOP("Box selection");

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 a pull request may close this issue.

2 participants