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

Custom Regex overrides can crash Atom #38

Open
willdady opened this issue Oct 31, 2014 · 8 comments
Open

Custom Regex overrides can crash Atom #38

willdady opened this issue Oct 31, 2014 · 8 comments

Comments

@willdady
Copy link

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

@DavidLGoldberg
Copy link
Owner

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.

@willdady
Copy link
Author

willdady commented Nov 3, 2014

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

@DavidLGoldberg
Copy link
Owner

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
Copy link
Owner

Easiest thing is to first take this stuff into like:

http://regexpal.com/

@DavidLGoldberg
Copy link
Owner

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 DavidLGoldberg changed the title I can crash Atom with Jumpy Custom Regex overrides can crash Atom Nov 6, 2014
@DavidLGoldberg
Copy link
Owner

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

@willdady
Copy link
Author

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

@DavidLGoldberg
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants