Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add ability to pull selected text into replace field #1064

Merged
merged 10 commits into from
Jan 10, 2019

Conversation

50Wliu
Copy link
Contributor

@50Wliu 50Wliu commented Dec 28, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This is an update of #719.

Alternate Designs

None.

Benefits

Allows the replace field to be easily populated with the selected text.

Possible Drawbacks

People may already be using ctrl-shift-e for other purposes.

Applicable Issues

#67.
Supersedes and closes #719.

Copy link
Contributor

@daviwil daviwil left a comment

Choose a reason for hiding this comment

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

All looks good except for one question!

lib/find-view.js Outdated
if (findPattern) {
this.findEditor.setText(findPattern);
this.search();
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this no longer activates the search immediately, is that the desired behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you're right, I thought this wasn't doing anything useful, but it turns out it does have a purpose if live search is disabled. I'll push a new commit fixing that.

@daviwil
Copy link
Contributor

daviwil commented Jan 10, 2019

Thanks @50Wliu! Looks good, merging it.

@daviwil daviwil merged commit a4cdbb6 into master Jan 10, 2019
@daviwil daviwil deleted the wl-ruthgrace-slurp-replace branch January 10, 2019 17:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants