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.

Sign up for GitHub

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

Jump to bottom

search-in-workspace: Perform workspace search in dirty file content #8579

Merged
merged 1 commit into from
Oct 30, 2020
Merged

search-in-workspace: Perform workspace search in dirty file content #8579

merged 1 commit into from
Oct 30, 2020

Conversation

DucNgn
Copy link
Contributor

@DucNgn DucNgn commented Oct 1, 2020 •
edited
Loading

What it does

Fixes: #5609
Closes: #5651

  • Search In Workspace can now search content in dirty files and display the results in search view.
  • Utilizes findMatches function from monaco editor to get the search matches.
  • The message alert in search view now only shows up when neither workspace nor editor present.

Note: PR #5651 can be safely closed if this PR get merged since this PR is an enhancement.

How to test

  • Open editor files in a workspace
  • Make changes in the editors without saving (make sure auto save is off)
  • Open Search-in-Workspace (ctrl/cmd + shift + f) and search for a certain keyword.
    • The results should correctly reflect the current state of the dirty editors.
    • The unsaved content in the editors should be shown in the search results, and properly highlighted in the editors.

searchInDirty2

Checklist when testing with search options

  • Match Case

  • Match Whole Word

  • Use Regular Expression

  • Include Ignored Files

  • files to include

  • files to exclude

Review checklist

Reminder for reviewers

Co-authored-by: fangnx naxin.fang@ericsson.com
Co-authored-by: vince-fugnitto vincent.fugnitto@ericsson.com
Signed-off-by: DukeNgn duc.a.nguyen@ericsson.com

@vince-fugnitto vince-fugnitto added the search in workspace issues related to the search-in-workspace label Oct 1, 2020
@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 3 times, most recently from fa98f7a to afabae9 Compare October 6, 2020 14:57
@DucNgn
Copy link
Contributor Author

DucNgn commented Oct 6, 2020

@vince-fugnitto Thanks for the reviews. I updated the PR 👍

@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 3 times, most recently from 54613b0 to 49fcf9b Compare October 9, 2020 20:00
@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 4 times, most recently from 6aea10b to 97aead7 Compare October 16, 2020 20:22
@DucNgn DucNgn mentioned this pull request Oct 16, 2020
Merged
1 task
@DucNgn
Copy link
Contributor Author

DucNgn commented Oct 20, 2020

@marechal-p I updated the PR to fix the issues you mentioned. Please review again when you have time, thanks :)

paul-marechal
paul-marechal reviewed Oct 21, 2020
View reviewed changes
Copy link
Member

@paul-marechal paul-marechal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested most button combinations, and only "Match Whole Words" didn't work.

packages/monaco/src/browser/monaco-editor.ts Outdated Show resolved Hide resolved
@paul-marechal
Copy link
Member

@akosyakov Would you move the findMatches function into the TextEditorDocument interface or keep it in the TextEditor one? Right now it is defined in the latter, but we're wondering if we should move it to TextEditorDocument? I don't see precisely which component should be responsible for what.

@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 4 times, most recently from 263fa63 to 9de6563 Compare October 26, 2020 15:19
paul-marechal
paul-marechal reviewed Oct 26, 2020
View reviewed changes
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

The feature works correctly and code LGTM. Please squash your commits.

packages/editor/src/browser/editor-preferences.ts Outdated Show resolved Hide resolved
@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 3 times, most recently from f479d0d to 9f1a4b4 Compare October 27, 2020 14:53
@DucNgn DucNgn force-pushed the dn/searchInDirtyContent branch 2 times, most recently from fe18673 to e1e69de Compare October 27, 2020 16:18
vince-fugnitto
vince-fugnitto approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes very well for me and the code looks good as well 👍
Thank you for the hard work on the pull-request!

@DucNgn @vince-fugnitto
siw: Perform workspace search in all open editors
7797a32 
+ `Search In Workspace` can now search content in dirty files and display the results in siw view.
+ Utilized the `findMatches` function from `monaco editor` to get the search matches from all open editors.
+ Added `minimatch` as a dependency in `siw`.
+ Change alert message in search view when no workspace present.
+ Only display alert in search view when neither workspace nor editor present.
+ Introduce `findMatches` method to `TextEditorDocument` interface.

Co-authored-by: fangnx <naxin.fang@ericsson.com>
Co-authored-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
Signed-off-by: DukeNgn <duc.a.nguyen@ericsson.com>
@paul-marechal paul-marechal merged commit 33886b5 into eclipse-theia:master Oct 30, 2020
@DucNgn DucNgn deleted the dn/searchInDirtyContent branch November 2, 2020 21:02
@paul-marechal paul-marechal added this to the 1.8.0 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-marechal paul-marechal paul-marechal left review comments

@vince-fugnitto vince-fugnitto vince-fugnitto approved these changes

Assignees
No one assigned
Labels
search in workspace issues related to the search-in-workspace
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.

[search-in-workspace] search does not respect dirty changes
3 participants
@DucNgn @paul-marechal @vince-fugnitto