The Wayback Machine - https://web.archive.org/web/20240824131714/https://github.com/atom/find-and-replace/pull/547
Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Catch scanInBufferRange errors #547

Merged
merged 9 commits into from
Feb 1, 2016
Merged

Catch scanInBufferRange errors #547

merged 9 commits into from
Feb 1, 2016

Conversation

50Wliu
Copy link
Contributor

@50Wliu 50Wliu commented Oct 6, 2015

This will catch RegExp too big errors and properly display them in the find box instead of a) displaying a uncaught exception, b) breaking tab switching, and c) (sometimes) preventing the find box from being usable.

TODO:

  • Figure out why the error isn't being displayed in the find box
  • Nicely format the error so that it's actually readable (RegExp too big should suffice) - changed to "Search string is too large"
  • Specs

Fixes #510
Fixes #532

@50Wliu
Copy link
Contributor Author

50Wliu commented Jan 31, 2016

I believe this is ready for review now - /cc @atom/feedback.

describe "when the search string is too large", ->
beforeEach ->
largeText = "abcdefghijklmnopqrstuvwxyz"
largeText += "abcdefghijklmnopqrstuvwxyz" for num in [0...2000]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to populate the find box with an absolutely massive search string? I believe when I first investigated this through trial and error the max regex length was 32768 (this method gives around 50000 characters just to be safe).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past, we've created a spec/fixtures folder and stored files there for large quantities of text. This way you can create a test file and just read from it without having to do stuff like this in the test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if the goal is just to create a long string, you should just do "x".repeat(50000).

This was accidentally added back in during the merge conflict I
think...it was removed in #583
@benogle benogle added the atom label Feb 1, 2016
@benogle
Copy link
Contributor

benogle commented Feb 1, 2016

Dope. Thanks @50Wliu!

benogle added a commit that referenced this pull request Feb 1, 2016
Catch scanInBufferRange errors
@benogle benogle merged commit 8af4eaf into master Feb 1, 2016
50Wliu added a commit that referenced this pull request Feb 1, 2016
Forgot to push this to #547 before it was merged
@50Wliu 50Wliu deleted the wl-regex-warning branch February 1, 2016 20:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
4 participants