-
Notifications
You must be signed in to change notification settings - Fork 27
Message logger #63
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
base: develop
Are you sure you want to change the base?
Message logger #63
Conversation
|
I like the idea of this, silent problems that are swallowed are annoying for the user. But isn't showing a message box an implementation detail that we should leave to the developer? Could this also be an event? Most users don't need a stack trace. I've never worked on logging on SolidDNA and I don't even know if it works, see #44. I also don't know if it shares data between two add-ins, which would be problematic. So I can't properly judge this PR yet. Questions:
Remarks:
|
I agree. I can add a setting property to toggle stack trace visibility.
I’m not sure which event you’re referring to. Could you clarify?
Agreed—I’ll make it optional.
It’s straightforward—similar to Microsoft’s logging library. SolidDNA’s static Logger class supports pluggable loggers (currently file-based, and this message logger soon). I’ve used it in two internal add-ins by wrapping it to MS logger and use it in DI container, and it works well.
It doesn’t share data between add-ins. Using different SolidDNA versions only overlap would be duplicate console messages. For message boxes, conflicts are rare (e.g., if errors occur in shared SolidDNA code). Even then, duplicate messages would likely be the least of the user’s concerns. In production, log level below Error is not used.
A scope groups all messages logged during its lifetime (from creation to disposal) into a single message box. This avoids spamming users with separate popups for each inner method’s warnings/errors.
This follows standard patterns (e.g., MS logging/DI). IDisposable marks an object’s lifetime—clean and brace-free with new using.
Order ensures messages display chronologically. The stack tracks nested scopes, the hashset prevents duplicates.
As mentioned in other PRs: if you merge the Command Manager PR first, this won’t be needed. Otherwise, I can create a fresh PR.
I nested these classes to avoid cluttering autocomplete—they’re only used by the parent class. If you prefer separate files, I’ll move them.
Understood. Just let me know whether to proceed with this PR or a new one, and I’ll apply fixes immediately. |
Added simple message logger to display logs in message boxes. Helps avoid silent failures by showing unhandled exceptions and important warnings to users.