-
Notifications
You must be signed in to change notification settings - Fork 407
Find and replace integration #1922
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1922 +/- ##
==========================================
+ Coverage 92.1% 92.12% +0.01%
==========================================
Files 189 189
Lines 10783 10809 +26
Branches 1580 1581 +1
==========================================
+ Hits 9932 9958 +26
Misses 851 851
Continue to review full report at Codecov.
|
Mostly copy and paste, like the implementations :thinking-face:
annthurium
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very cool, this will be super useful to our users.
Does this gracefully fall back if a user was somehow using an older version of find-and-replace?
| this.hasTerminatedPendingState = false; | ||
| this.refInitialFocus = new RefHolder(); | ||
|
|
||
| this.refEditor = new RefHolder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean about cleaning up our items so they aren't so repetitive.
Yup, both ways 👍
|
Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.
Requirements
Description of the Change
Implement the
observeEmbeddedTextEditor()protocol established in atom/find-and-replace#1069 (released in [email protected]) to support the "find" dialog in diff editors.Screenshot/Gif
Alternate Designs
N/A
Benefits
Large diffs become much more useful when you can find changes within them.
Possible Drawbacks
Attempting to trigger a "replace" causes a stacktrace in the console presently, because our TextEditors are read-only. I'd rather fix this in find-and-replace by disabling the replace controls though.
Applicable Issues
In concert with atom/find-and-replace#1069.
Metrics
N/A
Tests
Documentation
N/A
Release Notes
User Experience Research (Optional)
N/A