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

Updated main -> gui -> source -> main.cpp #2012

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Isaadqurashi
Copy link

Exception handling was added to catch any runtime errors.
Safe handling of the Linux distribution information was implemented to avoid potential crashes when the value is unavailable...
Improved logging for the Linux distribution to handle empty versions and missing distro data.

Exception handling was added to catch any runtime errors.
Safe handling of the Linux distribution information was implemented to avoid potential crashes when the value is unavailable...
Improved logging for the Linux distribution to handle empty versions and missing distro data.
@WerWolv
Copy link
Owner

WerWolv commented Dec 18, 2024

Thanks for the PR! I'm generally okay with the changes. However, as with the try-catch block in window.cpp, it should be #if-defined out in debug builds as it will make it very annoying to debug uncaught exceptions.

Additionally, instead of just logging to the console, it might be nice to display an OS popup using hex::nativeErrorMessage that shows the error message to the user

@Isaadqurashi
Copy link
Author

Thank you for the feedback!
I understand your concern about debugging uncaught exceptions in debug builds. I'll make sure to conditionally include the try-catch block using #if defined to ensure it's excluded in debug builds, making debugging more straightforward.

Regarding the logging mechanism I agree that displaying an OS popup using hex::nativeErrorMessage would provide a better user experience by immediately notifying the user of the error. I'll update the implementation accordingly...

However my finals are approaching, so I might need some time to make these adjustments. I'll ensure the changes are pushed as soon as possible after my exams. Please let me know if there's anything else you'd like me to consider in the meantime.

@WerWolv
Copy link
Owner

WerWolv commented Dec 18, 2024

No rush! I'll keep the the PR own until you're done :) Thanks a lot

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 this pull request may close these issues.

None yet

2 participants