From e3702ac2d88b1975714b007c71d2c48d648be4a2 Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sat, 20 May 2023 21:11:45 +0200 Subject: [PATCH] Console: Extend framework with intended recipient and content type metainformation ================================================================================== Limitations of the current framework: - Codes the translated state only for TranslatedNotification as part of the type. - Does not code the intended recipient (user, developer, ...) Problems: - Some errors are intended for developers, some errors may only be intended for users, if, for example, there is another developer error which already contains all the information. The current framework may lead to information duplication or to showing to the user developer information, which is perceived as annoying. - Logs shall be in English (report view), while every message to the user (UI) shall be translated. The current framework can only differentiate where to report based on subscription (legacy logs do not subscribe to notifications), and for notifications, whether it is translated or not depends on the type. It is not possible to code errors or warnings that are already translated. Solution: - To extend the ILogger interface with additional metainformation, indicating the intended recipient (User, Developer, All), and the content of the message (translated, untranslated, untranslatable). The latter is useful for dynamic strings that won't find a match in the translation framework. Bonus: - This extended version allows to do away with translatednotification, as now any message can be independently marked as translated or untranslated or untraslatable. - It is now possible to provide the right icon of severity (error, warning, info), even when it is only user intended and already translated. --- src/Base/Builder3D.cpp | 2 +- src/Base/Console.cpp | 31 +++++++++++-------- src/Base/Console.h | 55 ++++++++++++++++++++++++--------- src/Base/ConsoleObserver.cpp | 10 ++++-- src/Base/ConsoleObserver.h | 6 ++-- src/Gui/CommandTest.cpp | 5 ++- src/Gui/GuiConsole.cpp | 12 +++++-- src/Gui/GuiConsole.h | 3 +- src/Gui/MainWindow.cpp | 5 ++- src/Gui/MainWindow.h | 3 +- src/Gui/NotificationArea.cpp | 11 ++++--- src/Gui/ReportView.cpp | 5 ++- src/Gui/ReportView.h | 3 +- src/Gui/Splashscreen.cpp | 5 ++- src/Mod/Test/Gui/AppTestGui.cpp | 6 +++- 15 files changed, 115 insertions(+), 47 deletions(-) diff --git a/src/Base/Builder3D.cpp b/src/Base/Builder3D.cpp index 188a7a22de..25cfe518ba 100644 --- a/src/Base/Builder3D.cpp +++ b/src/Base/Builder3D.cpp @@ -987,7 +987,7 @@ void Builder3D::saveToLog() { ILogger* obs = Base::Console().Get("StatusBar"); if (obs){ - obs->SendLog("Builder3D",result.str().c_str(), Base::LogStyle::Log); + obs->SendLog("Builder3D",result.str().c_str(), Base::LogStyle::Log, Base::IntendedRecipient::Developer, Base::ContentType::Untranslatable); } } diff --git a/src/Base/Console.cpp b/src/Base/Console.cpp index c9c0c58321..bb26531731 100644 --- a/src/Base/Console.cpp +++ b/src/Base/Console.cpp @@ -49,11 +49,14 @@ namespace Base { class ConsoleEvent : public QEvent { public: ConsoleSingleton::FreeCAD_ConsoleMsgType msgtype; + IntendedRecipient recipient; + ContentType content; std::string notifier; std::string msg; - ConsoleEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, const std::string& notifier, const std::string& msg) - : QEvent(QEvent::User), msgtype(type), notifier(notifier),msg(msg) + ConsoleEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, IntendedRecipient recipient, + ContentType content, const std::string& notifier, const std::string& msg) + : QEvent(QEvent::User), msgtype(type), recipient(recipient), content(content), notifier(notifier), msg(msg) { } ~ConsoleEvent() override = default; @@ -77,25 +80,25 @@ public: ConsoleEvent* ce = static_cast(ev); switch (ce->msgtype) { case ConsoleSingleton::MsgType_Txt: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Message, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_Log: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Log, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_Wrn: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Warning, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_Err: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Error, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_Critical: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Critical, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_Notification: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::Notification, ce->recipient, ce->content, ce->notifier, ce->msg); break; case ConsoleSingleton::MsgType_TranslatedNotification: - Console().Notify(ce->notifier, ce->msg); + Console().notifyPrivate(LogStyle::TranslatedNotification, ce->recipient, ce->content, ce->notifier, ce->msg); break; } } @@ -286,18 +289,20 @@ void ConsoleSingleton::DetachObserver(ILogger *pcObserver) _aclObservers.erase(pcObserver); } -void Base::ConsoleSingleton::notifyPrivate(LogStyle category, const std::string& notifiername, const std::string& msg) +void Base::ConsoleSingleton::notifyPrivate(LogStyle category, IntendedRecipient recipient, + ContentType content, const std::string& notifiername, const std::string& msg) { for (std::set::iterator Iter=_aclObservers.begin();Iter!=_aclObservers.end();++Iter) { if ((*Iter)->isActive(category)) { - (*Iter)->SendLog(notifiername, msg, category); // send string to the listener + (*Iter)->SendLog(notifiername, msg, category, recipient, content); // send string to the listener } } } -void ConsoleSingleton::postEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, const std::string& notifiername, const std::string& msg) +void ConsoleSingleton::postEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, IntendedRecipient recipient, + ContentType content, const std::string& notifiername, const std::string& msg) { - QCoreApplication::postEvent(ConsoleOutput::getInstance(), new ConsoleEvent(type, notifiername, msg)); + QCoreApplication::postEvent(ConsoleOutput::getInstance(), new ConsoleEvent(type, recipient, content, notifiername, msg)); } ILogger *ConsoleSingleton::Get(const char *Name) const diff --git a/src/Base/Console.h b/src/Base/Console.h index 158fe1cc2d..f7b68ee011 100644 --- a/src/Base/Console.h +++ b/src/Base/Console.h @@ -474,6 +474,18 @@ enum class LogStyle{ TranslatedNotification, // Special message for already translated notifications to the user (e.g. educational) }; +enum class IntendedRecipient { + All, // All recipients, Developers and Users (One notification covers all) + User, // User only (notification intended only for a user) + Developer, // Developer only (notification intended only for a developer) +}; + +enum class ContentType { + Untranslated, // Not translated, but translatable + Translated, // Already translated + Untranslatable, // Cannot and should not be translated (Dynamic content, trace,...) +}; + /** The Logger Interface * This class describes an Interface for logging within FreeCAD. If you want to add a new * "sink" to FreeCAD's logging mechanism, then inherit this class. You'll also need to @@ -490,7 +502,8 @@ public: /** Used to send a Log message at the given level. */ - virtual void SendLog(const std::string& notifiername, const std::string& msg, LogStyle level) = 0; + virtual void SendLog(const std::string& notifiername, const std::string& msg, LogStyle level, + IntendedRecipient recipient, ContentType content) = 0; /** * Returns whether a LogStyle category is active or not @@ -559,7 +572,10 @@ public: Notification can be direct or via queue. */ - template + template inline void Send( const std::string & notifiername, const char * pMsg, Args&&... args ); /// Prints a Message @@ -587,19 +603,19 @@ public: /// Prints a Message with source indication template - inline void Message (const std::string &, const char * pMsg, Args&&... args); + inline void Message (const std::string & notifier, const char * pMsg, Args&&... args); /// Prints a warning Message with source indication template - inline void Warning (const std::string &, const char * pMsg, Args&&... args); + inline void Warning (const std::string & notifier, const char * pMsg, Args&&... args); /// Prints a error Message with source indication template - inline void Error (const std::string &, const char * pMsg, Args&&... args); + inline void Error (const std::string & notifier, const char * pMsg, Args&&... args); /// Prints a log Message with source indication template - inline void Log (const std::string &, const char * pMsg, Args&&... args); + inline void Log (const std::string & notifier, const char * pMsg, Args&&... args); /// Prints a Critical Message with source indication template - inline void Critical (const std::string &, const char * pMsg, Args&&... args); + inline void Critical (const std::string & notifier, const char * pMsg, Args&&... args); /// Sends a User Notification with source indication template inline void UserNotification( const std::string & notifier, const char * pMsg, Args&&... args ); @@ -608,7 +624,9 @@ public: inline void UserTranslatedNotification( const std::string & notifier, const char * pMsg, Args&&... args ); // Notify a message directly to observers - template + template inline void Notify(const std::string & notifiername, const std::string & msg); /// Attaches an Observer to FCConsole @@ -691,8 +709,10 @@ protected: virtual ~ConsoleSingleton(); private: - void postEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, const std::string& notifiername, const std::string& msg); - void notifyPrivate(LogStyle category, const std::string& notifiername, const std::string& msg); + void postEvent(ConsoleSingleton::FreeCAD_ConsoleMsgType type, IntendedRecipient recipient, + ContentType content, const std::string& notifiername, const std::string& msg); + void notifyPrivate(LogStyle category, IntendedRecipient recipient, ContentType content, + const std::string& notifiername, const std::string& msg); // singleton static void Destruct(); @@ -873,26 +893,31 @@ inline void Base::ConsoleSingleton::Log( const std::string & notifier, const cha Send(notifier, pMsg, std::forward(args)...); } -template +template inline void Base::ConsoleSingleton::Send( const std::string & notifiername, const char * pMsg, Args&&... args ) { std::string format = fmt::sprintf(pMsg, args...); if (connectionMode == Direct) { - Notify(notifiername,format); + Notify(notifiername,format); } else { auto type = getConsoleMsg(category); - postEvent(type, notifiername, format); + postEvent(type, recipient, contenttype, notifiername, format); } } -template +template inline void Base::ConsoleSingleton::Notify(const std::string & notifiername, const std::string & msg) { - notifyPrivate(category, notifiername, msg); + notifyPrivate(category, recipient, contenttype, notifiername, msg); } #if defined(__clang__) diff --git a/src/Base/ConsoleObserver.cpp b/src/Base/ConsoleObserver.cpp index dcce9091e8..d26509862b 100644 --- a/src/Base/ConsoleObserver.cpp +++ b/src/Base/ConsoleObserver.cpp @@ -58,9 +58,12 @@ ConsoleObserverFile::~ConsoleObserverFile() cFileStream.close(); } -void ConsoleObserverFile::SendLog(const std::string& notifiername, const std::string& msg, LogStyle level) +void ConsoleObserverFile::SendLog(const std::string& notifiername, const std::string& msg, LogStyle level, + IntendedRecipient recipient, ContentType content) { (void) notifiername; + (void) recipient; + (void) content; std::string prefix; switch(level){ @@ -101,9 +104,12 @@ ConsoleObserverStd::ConsoleObserverStd() : ConsoleObserverStd::~ConsoleObserverStd() = default; -void ConsoleObserverStd::SendLog(const std::string& notifiername, const std::string& msg, LogStyle level) +void ConsoleObserverStd::SendLog(const std::string& notifiername, const std::string& msg, LogStyle level, + IntendedRecipient recipient, ContentType content) { (void) notifiername; + (void) recipient; + (void) content; switch(level){ case LogStyle::Warning: diff --git a/src/Base/ConsoleObserver.h b/src/Base/ConsoleObserver.h index 4d510664f1..264933b1d6 100644 --- a/src/Base/ConsoleObserver.h +++ b/src/Base/ConsoleObserver.h @@ -42,7 +42,8 @@ public: explicit ConsoleObserverFile(const char *sFileName); ~ConsoleObserverFile() override; - void SendLog(const std::string& notifiername, const std::string& message, LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, LogStyle level, + IntendedRecipient recipient, ContentType content) override; const char* Name() override {return "File";} protected: @@ -57,7 +58,8 @@ class BaseExport ConsoleObserverStd: public ILogger public: ConsoleObserverStd(); ~ConsoleObserverStd() override; - void SendLog(const std::string& notifiername, const std::string& message, LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, LogStyle level, + IntendedRecipient recipient, ContentType content) override; const char* Name() override {return "Console";} protected: bool useColorStderr; diff --git a/src/Gui/CommandTest.cpp b/src/Gui/CommandTest.cpp index 5e867f94da..21b3e71e4e 100644 --- a/src/Gui/CommandTest.cpp +++ b/src/Gui/CommandTest.cpp @@ -728,9 +728,12 @@ public: TestConsoleObserver() : matchMsg(0), matchWrn(0), matchErr(0), matchLog(0), matchCritical(0) { } - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override{ + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override{ (void) notifiername; + (void) recipient; + (void) content; QMutexLocker ml(&mutex); diff --git a/src/Gui/GuiConsole.cpp b/src/Gui/GuiConsole.cpp index 764343979c..ef36b35bbb 100644 --- a/src/Gui/GuiConsole.cpp +++ b/src/Gui/GuiConsole.cpp @@ -81,10 +81,15 @@ GUIConsole::~GUIConsole (void) FreeConsole(); } -void GUIConsole::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) +void GUIConsole::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) { (void) notifiername; + // Do not log translated messages, or messages intended only to the user to std log + if(recipient == Base::IntendedRecipient::User || content == Base::ContentType::Translated) + return; + int color = -1; switch(level){ case Base::LogStyle::Warning: @@ -116,9 +121,12 @@ void GUIConsole::SendLog(const std::string& notifiername, const std::string& msg // safely ignore GUIConsole::s_nMaxLines and GUIConsole::s_nRefCount GUIConsole::GUIConsole () {} GUIConsole::~GUIConsole () {} -void GUIConsole::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) +void GUIConsole::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) { (void) notifiername; + (void) recipient; + (void) content; switch(level){ case Base::LogStyle::Warning: diff --git a/src/Gui/GuiConsole.h b/src/Gui/GuiConsole.h index 3633b6a685..1e27d437cd 100644 --- a/src/Gui/GuiConsole.h +++ b/src/Gui/GuiConsole.h @@ -47,7 +47,8 @@ public: GUIConsole(); /// Destructor ~GUIConsole() override; - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override; const char* Name() override {return "GUIConsole";} protected: diff --git a/src/Gui/MainWindow.cpp b/src/Gui/MainWindow.cpp index 3489b4c966..e7b866323a 100644 --- a/src/Gui/MainWindow.cpp +++ b/src/Gui/MainWindow.cpp @@ -2252,9 +2252,12 @@ void StatusBarObserver::OnChange(Base::Subject &rCaller, const char } } -void StatusBarObserver::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) +void StatusBarObserver::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) { (void) notifiername; + (void) recipient; + (void) content; int messageType = -1; switch(level){ diff --git a/src/Gui/MainWindow.h b/src/Gui/MainWindow.h index aea0570cac..ca40c48b9f 100644 --- a/src/Gui/MainWindow.h +++ b/src/Gui/MainWindow.h @@ -370,7 +370,8 @@ public: /** Observes its parameter group. */ void OnChange(Base::Subject &rCaller, const char * sReason) override; - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override; /// name of the observer const char *Name() override {return "StatusBar";} diff --git a/src/Gui/NotificationArea.cpp b/src/Gui/NotificationArea.cpp index 141d3580c4..727e76070c 100644 --- a/src/Gui/NotificationArea.cpp +++ b/src/Gui/NotificationArea.cpp @@ -214,8 +214,8 @@ public: /// Function that is called by the console interface for this observer with the message /// information - void SendLog(const std::string& notifiername, const std::string& msg, - Base::LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override; /// Name of the observer const char* Name() override @@ -242,8 +242,8 @@ NotificationAreaObserver::~NotificationAreaObserver() Base::Console().DetachObserver(this); } -void NotificationAreaObserver::SendLog(const std::string& notifiername, const std::string& msg, - Base::LogStyle level) +void NotificationAreaObserver::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) { // 1. As notification system is shared with report view and others, the expectation is that any // individual error and warning message will end in "\n". This means the string must be stripped @@ -253,6 +253,9 @@ void NotificationAreaObserver::SendLog(const std::string& notifiername, const st // "\n", as this generates problems with the translation system. Then the string must be // stripped of "\n" before translation. + (void) recipient; + (void) content; + auto simplifiedstring = QString::fromStdString(msg) .trimmed();// remove any leading and trailing whitespace character ('\n') diff --git a/src/Gui/ReportView.cpp b/src/Gui/ReportView.cpp index c3eeb97ac2..a9427cb48d 100644 --- a/src/Gui/ReportView.cpp +++ b/src/Gui/ReportView.cpp @@ -472,9 +472,12 @@ void ReportOutput::restoreFont() setFont(serifFont); } -void ReportOutput::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) +void ReportOutput::SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) { (void) notifiername; + (void) recipient; + (void) content; ReportHighlighter::Paragraph style = ReportHighlighter::LogText; switch (level) { diff --git a/src/Gui/ReportView.h b/src/Gui/ReportView.h index 3255558051..276284ca9f 100644 --- a/src/Gui/ReportView.h +++ b/src/Gui/ReportView.h @@ -141,7 +141,8 @@ public: /** Observes its parameter group. */ void OnChange(Base::Subject &rCaller, const char * sReason) override; - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override; + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override; /// returns the name for observer handling const char* Name() override {return "ReportOutput";} diff --git a/src/Gui/Splashscreen.cpp b/src/Gui/Splashscreen.cpp index abd50fa9dd..87c1c81bc2 100644 --- a/src/Gui/Splashscreen.cpp +++ b/src/Gui/Splashscreen.cpp @@ -176,9 +176,12 @@ public: { return "SplashObserver"; } - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override { Q_UNUSED(notifiername) + Q_UNUSED(recipient) + Q_UNUSED(content) #ifdef FC_DEBUG Log(msg.c_str()); diff --git a/src/Mod/Test/Gui/AppTestGui.cpp b/src/Mod/Test/Gui/AppTestGui.cpp index 261ee5b67b..5b69652b79 100644 --- a/src/Mod/Test/Gui/AppTestGui.cpp +++ b/src/Mod/Test/Gui/AppTestGui.cpp @@ -40,9 +40,13 @@ public: void flush() {buffer.str("");buffer.clear();} - void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level) override{ + void SendLog(const std::string& notifiername, const std::string& msg, Base::LogStyle level, + Base::IntendedRecipient recipient, Base::ContentType content) override{ (void) notifiername; (void) msg; + (void) recipient; + (void) content; + switch(level){ case Base::LogStyle::Warning: buffer << "WRN";