Skip to content

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

Merged
merged 8 commits into from
Feb 16, 2020
Merged

Conversation

BlueDrink9
Copy link
Collaborator

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.

bluedrink9 added 3 commits December 10, 2019 15:58
@BlueDrink9
Copy link
Collaborator Author

What's our logging system?

@luziferius
Copy link
Contributor

luziferius commented Dec 10, 2019

AutoKey uses the standard library logging module. But I forgot that it is not that well used in AutoKey.
For the Qt GUI, you can grab a logger instance from autokey.qtui.common. I looked around and it seems that the GTK GUI does no logging at all. So you can disregard my previous comment about logging it.
Just a TODO: Add logging should be good enough. I’ll refactor the logging system used by AutoKey in the future, as that’s something on my roadmap anyways.

To track the logging, I opened #338

@luziferius
Copy link
Contributor

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 master and wrap the lasted fixes up into 0.95.10

@BlueDrink9
Copy link
Collaborator Author

BlueDrink9 commented Feb 11, 2020

I'll take a look. Does the new logging system have any automated integration with the guis?

@luziferius
Copy link
Contributor

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.

@BlueDrink9
Copy link
Collaborator Author

Ok. Might it be worth looking at adding a handler that alerts via gui?

@luziferius
Copy link
Contributor

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.

@BlueDrink9
Copy link
Collaborator Author

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.

@luziferius luziferius linked an issue Feb 16, 2020 that may be closed by this pull request
@luziferius luziferius merged commit 51bc0f1 into autokey:develop Feb 16, 2020
@luziferius
Copy link
Contributor

Merged and cherry-picked the crash mitigation into the master branch.

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.

Fatal error if user enters an invalid regex
2 participants