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

Exceptions: Enable translations of dynamic strings #9709

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

abdullahtahiriyo
Copy link
Contributor

This is to solve this kind of issues:
FreeCAD/FreeCAD-translations#183

This PR is a mechanism to allow to mark for translation dynamic strings in Exceptions and in particular, enabling Python exceptions to use such dynamic strings.

A piece of code is worth a thousand words:

  PyError_SetEnhancedString(
      PyExc_ValueError,
      QT_TRANSLATE_NOOP("Notifications",
                        "Datum {} for the constraint with index {}  is invalid"),
      (const char*)Quantity.getUserString().toUtf8(),
      Index);

This is a PyErr_SetString substitute using fmt::format syntax, which stores the formatter string and the arguments separately as strings in the exception. In Python this is done with an exception dictionary object. In c++ Base::Exception is extended to store it. It also marks the flag that is it is a translatable string.

With PyErr_SetString only static strings (without substitutions are possible).

Because the Exception error message is untranslated, any exception reporting to the Report View at the Interpreter is provided in the original English language (but formatted by fmt::format).

At the other end, the Notifications Framework is extended so that it can take an exception as a notification message:

catch (const Base::Exception& e) {
            Gui::NotifyUserError(sketch, QT_TRANSLATE_NOOP("Notifications", "Value Error"), e);
        }

It will translate the formatter string and arguments as necessary, assemble the translation (by fmt::format), and report it to the notification area or blocking pop-up as per user preferences.

=============================================================

Problem:
When strings are dynamically generated, it is currently not possible
to mark them for translation using QT_TRANSLATE_NOOP. The reason is
that the string marked with the macro will change (for example by
taking dynamically generated parameters).

The problem is that these strings will not be translated.

Solution:
- To extend Exception to store a formatter string suitable for
fmt::format, and string representations of the parameters.
- To extend Exceptions to be provided with a translator function
to assemble a translated error message when such translation is
necessary.
…input

==========================================================================

Problem:
When running interactive commands directly from the console (i.e. writting commands in the console),
Python exceptions having a dictionary (e.g. those raised with PyErr_SetObject and setting a PyDict as
object) are processed via a not thrown RuntimeError exception.

The current version disregards the what message of Exceptions with dictionary input, as the RunTimeError
is not initialised with it, and setPyObject does not handle the "swhat" dictionary input.

The outcome is that the generic FreeCAD message is printed:
>>> App.getDocument('partiallyRed').getObject('Sketch002').setDatum(14,App.Units.Quantity('-10.000000 mm'))
Traceback (most recent call last):
  File "<input>", line 1, in <module>
ValueError: FreeCAD Exception

Solution:

To feed the RunTimeError exception with the swhat message, so that the exception is much more informative:

>>> App.getDocument('partiallyRed').getObject('Sketch002').setDatum(14,App.Units.Quantity('-10.000000 mm'))
Traceback (most recent call last):
  File "<input>", line 1, in <module>
ValueError: Datum -10,000 mm for the constraint with index 14  is invalid
=================================================================

Now that Exceptions have been extended to enable the translation of dynamic
strings, it makes sense to extend the notifications framework to pass
the exception directly and let the framework handle translations and raw
English messages as necessary (for the Report View and the pop-ups / Notification
Area).

This commmit:
- Creates a translator function necessary to enable Exceptions to
translate themselves.
- Extends the framework to take as message, in addition to a string,
an exception.
================================

Utility functions to manage dictionary based enhanced Python Exceptions.

PyError_SetEnhancedString is a convenience function to enable dynamic
strings that can be translated. It takes fmt::format syntax and stores
formatter and arguments separatedly as a dictionary. It also sets the
boolean flag that is translatable in the dictionary.
@github-actions github-actions bot added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB Sketcher Related to the Sketcher Workbench labels Jun 1, 2023
d.setItem("btranslatable", Py::Boolean(true));

PyErr_SetObject(exceptiontype, Py::new_reference_to(d));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wwmayer

I would welcome a trained second set of eyes on this function and in particular on whether I may be leaking (ref counter issues).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me.

@luzpaz luzpaz added the Translation Translation and localization related label Jun 11, 2023
@abdullahtahiriyo abdullahtahiriyo marked this pull request as draft June 11, 2023 16:36
@abdullahtahiriyo
Copy link
Contributor Author

I will not be available for some time, so I think it is best to turn this into draft.

@abdullahtahiriyo abdullahtahiriyo added the 0.22 for the v0.22 development cycle label Jun 11, 2023
@freecadci
Copy link

pipeline status for feature branch PR_9709. Pipeline 903580490 was triggered at 9e16b48. All CI branches and pipelines.

@sliptonic sliptonic added this to the 1.0 milestone Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.22 for the v0.22 development cycle Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Translation Translation and localization related WB Sketcher Related to the Sketcher Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants