-
Notifications
You must be signed in to change notification settings - Fork 213
Fix #212 crash on invalid regex #336
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
Conversation
Don't actually need them, but I wrote them so I'll leave them for now.
What's our logging system? |
AutoKey uses the standard library To track the logging, I opened #338 |
Can we get this in? I think catching the exception, so that it does not crash is ok for now. I can then cherry-pick it into |
I'll take a look. Does the new logging system have any automated integration with the guis? |
Nope. It currently works per module. So just create a logger instance at the top (#338 (comment)), if there isn't already one and you are good to go. |
Ok. Might it be worth looking at adding a handler that alerts via gui? |
Definitely. In the Qt abbreviation configuration dialog, the OK button only enables, if the content is actually valid. When the content is invalid, only Cancel is available. This removed the need for an error message box that was shown there previously. Something like disabling OK plus marking the invalid RE in red plus showing a description label should be good. |
Added logger. Also, I have a local commit that moves the contents of the 'save' functions (which are 100% shared between the two UI classes) to a separate file. I haven't pushed it to make it easier for you to merge this, but afterwards I'll need help writing the tests for them. |
Merged and cherry-picked the crash mitigation into the master branch. |
Absolutely still a work in progress, but for the time being I've silenced the crash. It just won't save the regex if it's invalid.
I don't know enough about the GUI to handle warning the user about the error.