PD NEXT: Mapping change and support

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PD NEXT: Mapping change and support

Post by abdullah »

wmayer wrote:I would prefer a special Exception class. Not sure if we need a whole set of classes for this or if one is sufficient.
wmayer wrote: As said above we don't need a translated and an untranslated version for each exception type. When you have a look at Exception.h then most of them are used to describe some pure technical problems. The only type which may also show informative text to the user is RuntimeError. So, for me it's fine to have only one exception class used for translation.
I was not referring to a full duplication. I was referring to some duplication. But I see your point.

The rest of this message is product of a brainstorming and I post it not to forget about it. Feel free to stop reading here.

The first question to answer is: Do we need to differentiate between different types of exceptions that offer or should offer a translatable text?

As particular examples of today I can think of:
1) OCC exception that is catch and thrown as a base exception, for example CADKernelError exception. In this case the translatable message is fixed and exception class defined "CAD kernel error".
2) A value error exception (I have been using the ValueError, e.g. "BSpline GeoId is out of bounds." but I can agree to any other name, it is generally just something the user entered wrongly, or an error in the UI command code). The translatable message is given by the user via the macro, though I have not tested it, probably the macro could pass the string thru QT_TRANSLATE_NOOP using a context like QObject (or any other), so that lupdate marks the text for translation. NOTE: We do not want to provide the translation itself when throwing, to be able to use the original English message for the report view.

As a general principle, the mechanism should serve the two purposes (allow for specialized catching, i.e. the reason why we do not use Base::Exception everywhere, and reducing the UI message generation), the mechanism should allow that any exception susceptible of generating a developer (exception class user) defined text message can be used as translatable (it does not necessarily need to know it is translatable, see below). A general workflow:

1. The developer has a function where different types of exceptions are thrown.
2. The developer has a good message for the UI on several of them (not necessarily all).
3. The developer uses the different types, so that at the moment of catching (elsewhere), he can differentiate between types.
4. At the moment of catching (elsewhere), appropriate action is taken for each type and the exception (the same or a specialized one) is rethrown for the UI.

One solution is to use two different macros for the initial throwing. One for when the message is translatable (Macro1), another for when it is not (Macro2). There are several possible implementations. One would be that Macro1 passes the text thru QT_TRANSLATE_NOOP and macro2 does not. In this implementation (4) can not know whether the text is translatable or not, but it could by default rethrow the exception for the UI. The UI has access to the translation engine, so it should be possible to detect whether the message was marked for translation or not (it was thrown via Macro1 or Macro2). If at runtime the message can not be found in the translation engine, then a default error is shown. If it can be found, the translation is shown.

Maybe this could work without any specific exception. I will try it and come back.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: PD NEXT: Mapping change and support

Post by DeepSOIC »

abdullah wrote:macros for the initial throwing
Why macro??
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PD NEXT: Mapping change and support

Post by abdullah »

DeepSOIC wrote:
abdullah wrote:macros for the initial throwing
Why macro??

Code: Select all

#define THROW(exception) throw Base::exception(__FILE__,__LINE__,__PRETTY_FUNCTION__);
#define THROWM(exception, message) throw Base::exception(message,__FILE__,__LINE__,__PRETTY_FUNCTION__);
Because it is the pre-processor the only one who knows about all that information.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PD NEXT: Mapping change and support

Post by abdullah »

Ah you mean, why macros for the translation message (which is not implemented, not tested, just an idea that may or may not work in any case).

The throwing is done via macro today in stage 1 because of the other reasons given.

There are two options, to allow the developer to do this:
THROWM(ValueError, QT_TRANSLATE_NOOP("QObject","Please mark this message for translation"))
THROWM(ValueError, "Too technical message no intended for the user, do not mark for translation"))

or having something like:
THROWMT(ValueError, "Please mark this message for translation")
THROWM(ValueError, "Too technical message no intended for the user, do not mark for translation"))
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: PD NEXT: Mapping change and support

Post by DeepSOIC »

abdullah wrote:Because it is the pre-processor the only one who knows about all that information.
Thanks, is was instantly clear as soon as I saw the code ;) .
ian.rees
Posts: 696
Joined: Sun Jun 15, 2014 3:28 am
Contact:

Re: PD NEXT: Mapping change and support

Post by ian.rees »

Admittedly, I haven't read through the entire threads behind this - but it seems we're re-inventing the wheel by trying to maintain context from the thrower, all the way up. Have we looked in to capturing the stack and reporting that instead of just a line number? I think there are libraries that do this - one crash reporter (done by the Chrome folks IIRC) was discussed on here a few weeks ago. A stack trace provides much more useful information than just the throw point.

I don't think it's a good idea to do translation in the throw. What if something in the string handling throws? Much better to have a static C string that's greppable, and have the catcher provide nice user-facing error messages. -Ian-
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: PD NEXT: Mapping change and support

Post by abdullah »

ian.rees wrote:but it seems we're re-inventing the wheel by trying to maintain context from the thrower, all the way up. Have we looked in to capturing the stack and reporting that instead of just a line number? I think there are libraries that do this - one crash reporter (done by the Chrome folks IIRC) was discussed on here a few weeks ago. A stack trace provides much more useful information than just the throw point.
Yes, it is more information, but it comes at a cost of dependencies and runtime performance. This implementation, which I would not regard as reinventing the wheel, because it is what several other projects use, is some kind of trade off.
ian.rees wrote:I don't think it's a good idea to do translation in the throw.
No, we are not translating in the throw. Yes, at compilation time there is a literal in every throw. You are just marking it for translation. QT_TRANSLATE_NOOP after the preprocesor is just the literal of the second parameter. It is there to allow the translation tools that work by reading the source to detect which literals need translating. Then at runtime, where a translation for such a string is needed and requested, the translation engine can provide one. This only happens in the UI part of FreeCAD (we do not translate the report view or the python console).

See this post and the next one:
https://forum.freecadweb.org/viewtopic. ... 20#p171816
ian.rees wrote:and have the catcher provide nice user-facing error messages.
The whole idea of the translation part is to avoid duplicating error messages between console, report view and UI. Today we are creating one message at App level and another one at GUI level (when it is done "user friendly" and we do not have a pop-up with function names and cryptic messages). There is no need to do duplicated work if it can be done right from the beginning. This is an attempt to provide a light framework to do it right from the beginning.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: PD NEXT: Mapping change and support

Post by wmayer »

I think there are libraries that do this - one crash reporter (done by the Chrome folks IIRC) was discussed on here a few weeks ago. A stack trace provides much more useful information than just the throw point.
I don't think that this works as expected for all platforms. On Windows there is e.g. the StackWalker which works nicely in Debug mode. But if you are running it in Release mode and if calling functions are not exported to dlls the retrieved function names gives nonsense. So, a stacktace might be more confusing than that it helps.
Post Reply