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

Find within TextEditors within non-editor pane items #1068

Merged
merged 3 commits into from Jan 25, 2019

Conversation

Projects
None yet
1 participant
@smashwilson
Copy link
Member

smashwilson commented Jan 25, 2019

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

In atom/github, we open several pane items that embed TextEditor elements, but are not TextEditors themselves: ChangedFileItems for single file diffs (stage and unchanged), CommitPreviewItems for commit previews, and the "Files Changed" tab within the pull request detail item. Cmd-F doesn't search them, which is inconvenient, because especially the last two can be large.

To accommodate them, and any other packages that created pane items with internal TextEditors, I'm allowing pane items to implement a getEmbeddedTextEditor() method. If present on the active pane item, when the find view is opened, that embedded item will be searched.

Alternate Designs

I thought about making the method getEmbeddedTextEditors(), plural, and allowing a pane item to return multiple editors and search across all of them. It would have been a fair bit more work and we didn't actually need it for atom/github, so I held off to keep the scope small.

Benefits

find-and-replace:show will be able to be used to search buffers embedded in custom pane items as well as core TextEditors.

Possible Drawbacks

It's possible that there are items in third-party packages that already implement a method with that name. Although this is kind of a bonus too?

Applicable Issues

N/A

smashwilson added some commits Jan 25, 2019

@smashwilson smashwilson merged commit 39e4e4c into master Jan 25, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@smashwilson smashwilson deleted the aw/find-in-embedded-editors branch Jan 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.