jumpy icon indicating copy to clipboard operation
jumpy copied to clipboard

Custom Regex overrides can crash Atom

Open willdady opened this issue 11 years ago • 8 comments

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.

([A-Z]+([0-9a-z])*)|[a-z0-9]{2,}|^\s*$

Windows v0.140.0

willdady avatar Oct 31 '14 02:10 willdady

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 avatar Nov 03 '14 02:11 DavidLGoldberg

@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.

willdady avatar Nov 03 '14 02:11 willdady

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: |\n That might work to achieve end of line + empty lines right?

DavidLGoldberg avatar Nov 03 '14 21:11 DavidLGoldberg

Easiest thing is to first take this stuff into like:

http://regexpal.com/

DavidLGoldberg avatar Nov 03 '14 21:11 DavidLGoldberg

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.

DavidLGoldberg avatar Nov 03 '14 21:11 DavidLGoldberg

Added the help wanted, because not sure how to fix the original issue here.

DavidLGoldberg avatar Mar 27 '15 05:03 DavidLGoldberg

@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.

willdady avatar Mar 27 '15 05:03 willdady

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?)

DavidLGoldberg avatar Mar 27 '15 11:03 DavidLGoldberg