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

Add Replace in Files command to Edit main-menu #10242

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

smart-bo
Copy link
Contributor

@smart-bo smart-bo commented Oct 6, 2021

What it does

Fixes #10217

The commit includes the following changes:
Add the replace in files command to the edit main-menu.
It will open the search-in-workspace with the replace field expanded. If a selection exists in the editor, the search input will be pre-populated.

How to test

  • start the application
  • confirm the replace in files menu item exists under the edit main-menu
  • confirm the replace in files command exists in the command palette
  • confirm the command will trigger the opening of the search-in-workspace view with the input pre-populated if a selection exists, and the replace field opened

Review checklist

Reminder for reviewers

@msujew msujew added the search in workspace issues related to the search-in-workspace label Oct 6, 2021
@msujew
Copy link
Member

msujew commented Oct 6, 2021

Hi @smart-bo, thank your for your contribution, the changes are looking quite promising.

Do you mind giving this PR a more descriptive title? Also, would you please populate the How to test section of your PR for us reviewers to verify the expected behavior of your changes?

@smart-bo smart-bo changed the title Gh 10217 Add Replace in Files command to the edit main-menu Oct 7, 2021
@smart-bo smart-bo changed the title Add Replace in Files command to the edit main-menu Add Replace in Files command to Edit main-menu Oct 7, 2021
@smart-bo
Copy link
Contributor Author

smart-bo commented Oct 7, 2021

Hi @smart-bo, thank your for your contribution, the changes are looking quite promising.

Do you mind giving this PR a more descriptive title? Also, would you please populate the How to test section of your PR for us reviewers to verify the expected behavior of your changes?

Thank you for your kind remind, the PR was modified, please check.

@smart-bo smart-bo force-pushed the gh-10217 branch 2 times, most recently from e5de644 to 0d83c58 Compare October 12, 2021 18:46
@smart-bo smart-bo force-pushed the gh-10217 branch 2 times, most recently from 13cc36a to beac550 Compare October 18, 2021 18:16
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Due to the introduction of localization in Theia for the upcoming release, you'll have to rebase your PR and apply the suggestion below to your code. Sorry for the inconvenience.

Afterwards the PR is ready to go!

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

@smart-bo Thanks, that looks good to me.

Regarding the replace field focus issue: I would recommend that we tackle this in a separate issue, since that is a concurrency problem when running the refresh method. The method automatically focuses the search field, but it does so in an async way (although it is not marked as async), which prevents us from awaiting the refresh and setting the focus to the replace field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
search in workspace issues related to the search-in-workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

search-in-workspace: add replace in files command
2 participants