-
Notifications
You must be signed in to change notification settings - Fork 16
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
Custom Regex overrides can crash Atom #38
Comments
Hmm. I think there are an infinite number of regexes that would do that :) I don't think I'm going to change anything here. Anyone overriding regexes should know they have the power to be very expensive etc. I might be able to put some kind of timeout, but that'd be a lot of bloat I don't think the code needs. What do you think, any suggestions on how to fix? Without diving too deep into that particular regex, do you think it should work? Did you try the same regex and test data on a regex tester and see if it comes back quickly? My guess is it crashes any other sort of regex tester you use with same regex + same test data. |
@DavidLGoldberg I haven't done much further testing. I was just trying to match blank lines in addition to the defaults. I can live without it. Do you think something like this should result in the entire editor crashing? Is it worth escalating this to the core Atom team. I don't know enough about Atom and packages to know if this is even something they can prevent. |
Awesome that you're trying that btw, It's on my list too! I also want to add a pointer to the end of the line and more symbols (as long as theyr'e like 2 chars wide) I might try this weekend, but did you try: |
Easiest thing is to first take this stuff into like: |
I'll try to take a look at this this weekend. I've wanted it for some time myself too. I can't remember all of the nuance of how the regex works, but usually I have 2 chars that I replace. It might end up being easiest to add code to handle the newlines, in either of the cases (end of line or empty line). The 2 random symbols like += for example should be easier to do with the custom / replace the default regex.. Also I'd like to have like {\n and }\n for c style languages at some point. |
Added the help wanted, because not sure how to fix the original issue here. |
@DavidLGoldberg I actually think you should not expose the regex as a setting at all (at least not in the UI). I think the default you have is fine provided you can also match the beginning and end of lines. I think this should cover 99% of use-cases. |
Last night I reached the same conclusion! BTW. Now that I've settled some of the issues I wanted to, I'm going to give another round of trying to pull in your pull request and then change it to the defaults as we discussed. I'll give it a shot this weekend. I'll leave this open too, until I lock down the regex. Do you think the lockdown warrants a "3.0" (technically it's a "breaking" change what do you think?) |
If I alter the match pattern on Jumpy's setting screen to the following and then try to invoke Jumpy the entire editor locks up and then crashes.
Windows v0.140.0
The text was updated successfully, but these errors were encountered: