Fixed #5454, #5452: modified "replace-all" to save changes without opening editors #5600

fangnx · 2019-06-27T19:18:31Z

In search-in-workspace widget, doing the "replace-all" used to open all the editor files in dirty state.
With this patch, "replace-all" will do all the text replacement, and simply save the changes in the editors without opening them.

vince-fugnitto

Very nice work! 👍 BTW, it also fixes #5452

vince-fugnitto · 2019-06-27T20:52:16Z

@lmcbout can you try the PR yourself as well?

vince-fugnitto · 2019-06-27T20:56:40Z

@fangnx one thing however, if you were to perform a search + replace for a single node, clicking that node in the tree to replace only for him should open the file with the changes.

See the behavior below in vscode:

updated testing.

lmcbout · 2019-06-28T14:28:42Z

When a file is opened, the replace all affects open and closed files and seems OK until the open file is in dirty state, then the replace all stops working. From that point on, you can add char in the file, but pressing "ctrl + s" to save the dirty file is not working anymore, replace all to return to its original data is not working anymore it is not detecting the new search.

.

fangnx · 2019-06-28T14:45:05Z

When a file is opened, the replace all affects open and closed files and seems OK until the open file is in dirty state, then the replace all stops working. From that point on, you can add char in the file, but pressing "ctrl + s" to save the dirty file is not working anymore, replace all to return to its original data is not working anymore it is not detecting the new search.

.

I will look into this ;)

fangnx · 2019-06-28T17:31:49Z

When a file is opened, the replace all affects open and closed files and seems OK until the open file is in dirty state, then the replace all stops working. From that point on, you can add char in the file, but pressing "ctrl + s" to save the dirty file is not working anymore, replace all to return to its original data is not working anymore it is not detecting the new search.

.

I updated the PR. With my new updates, I do not see the issue anymore.

packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx

vince-fugnitto · 2019-07-05T16:30:04Z

@lmcbout do you mind re-testing the PR?

lmcbout · 2019-07-08T19:16:51Z

When there is a dirty file, {Add an empty line } before you look for a string to replace, you end-up with corrupted data. Corrupted data will be fix with the the following issue #5609 / pr #5651
Note: replacing data from a dirty file was there before this patch. This patch only prevent opening all files in a dirty state.

lmcbout

Nice improvement
One thing, I noticed that if a file is open and is not formatted, (JAVA, TS, CPP)
1- Focus on the file in editor view when replace all occurs: file save and reformatted
2- File in editor, but the focus is on a different file, replace all : file saved but not re-formatted
The "editor.formatOnSave" preference issue is tracked with issue #5740.

akosyakov · 2019-07-18T06:23:46Z

packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx

+     */
+    protected async doGetWidget(node: SearchInWorkspaceResultLineNode): Promise<EditorWidget> {
+        const fileUri = new URI(node.fileUri);
+        const editorWidget = await this.editorManager.getOrCreateByUri(fileUri);


if it creates a new widget, at what point of time is gets disposed and garbage collected?

Could you check that there are no memory leaks with this change please in the case if this code creates a new instance.

if it creates a new widget, at what point of time is gets disposed and garbage collected?

Could you check that there are no memory leaks with this change please in the case if this code creates a new instance.

I thought it would be handled as it extends widget. Could you please tell me how I can do memory leak check for Theia?

I added a bunch of code inside this function and displayed all the editors tracked by the editor manager, it happens that editors are leaking indeed. Good thing is that we only leak one of the same URI, since getOrCreate won't spawn duplicates.

I don't know how you should go about disposing them, I also find weird that we have to instantiate EditorWidget in order to change a file and save it?

@akosyakov ?

I thought it would be handled as it extends widget. Could you please tell me how I can do memory leak check for Theia?

In dev tools, there is memory tab, you should force garbage collection first, then take a snapshot, and check whether there are no EditorWidget, except a constructor (you will see a special character next to such instance).

I don't know how you should go about disposing them, I also find weird that we have to instantiate EditorWidget in order to change a file and save it?

You should use ResourceProvider to get a Resource and then modify and dispose it if you want to modify content on the disk without editor or dirty model.

In the updated patch, I manually dispose the editor widget after it has been modified and saved, since editor widget has to be called in order to use the replaceText method. I tested with dev tools and could confirm that the editor widget could be disposed properly.

please ask someone to verify that:

it does not cause some glitches, like this editor also gets really opened in the meantime and then unexpectedly closed

there is no memory leak

If someone checks it and it is fine then it is fine to merge. Don't have time to do it now sorry

I looked and did not find a memleak, approving :)

…out opening editors - In `search-in-workspace` widget, doing the "replace-all" used to open all the editor files in dirty state. - With this patch, "replace-all" will do all the text replacement, and simply save the changes in the editors without opening them. Signed-off-by: fangnx <naxin.fang@ericsson.com>

paul-marechal

No memory leak, LGTM!

vince-fugnitto previously approved these changes Jun 27, 2019

View reviewed changes

vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace labels Jun 27, 2019

vince-fugnitto requested a review from lmcbout June 27, 2019 20:52

vince-fugnitto self-requested a review June 27, 2019 20:56

fangnx force-pushed the fnx-fix-5454 branch from b1d7246 to 1593f87 Compare June 28, 2019 17:21

fangnx changed the title ~~Fixed #5454: modified "replace-all" to save changes without opening editors~~ Fixed #5454, #5452: modified "replace-all" to save changes without opening editors Jun 28, 2019

fangnx force-pushed the fnx-fix-5454 branch from 1593f87 to c89e691 Compare June 28, 2019 17:26

akosyakov reviewed Jul 1, 2019

View reviewed changes

packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx Show resolved Hide resolved

akosyakov requested a review from jbicker July 1, 2019 08:46

lmcbout mentioned this pull request Jul 15, 2019

File in editor goes dirty after "replace all" and is not saveable after #5671

Closed

fangnx force-pushed the fnx-fix-5454 branch from c89e691 to bcdbe27 Compare July 16, 2019 15:12

vince-fugnitto mentioned this pull request Jul 17, 2019

Replace all can freeze all editors. #5740

Open

lmcbout approved these changes Jul 17, 2019

View reviewed changes

akosyakov reviewed Jul 18, 2019

View reviewed changes

fangnx force-pushed the fnx-fix-5454 branch from bcdbe27 to 1c53bfb Compare July 25, 2019 17:07

fangnx force-pushed the fnx-fix-5454 branch from 1c53bfb to aa46562 Compare July 25, 2019 17:09

paul-marechal approved these changes Aug 1, 2019

View reviewed changes

paul-marechal merged commit 2a65e63 into eclipse-theia:master Aug 1, 2019

vince-fugnitto mentioned this pull request Nov 22, 2019

[search-in-workspace] replace all should apply immediately without dirty state #5452

Closed

magnologan mentioned this pull request Jan 9, 2023

[Snyk] Security upgrade puppeteer from 2.1.1 to 3.0.0 magnologan/theia#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #5454, #5452: modified "replace-all" to save changes without opening editors #5600

Fixed #5454, #5452: modified "replace-all" to save changes without opening editors #5600

fangnx commented Jun 27, 2019 •

edited

Loading

vince-fugnitto left a comment

vince-fugnitto commented Jun 27, 2019 •

edited

Loading

vince-fugnitto commented Jun 27, 2019

lmcbout commented Jun 28, 2019

fangnx commented Jun 28, 2019

fangnx commented Jun 28, 2019

vince-fugnitto commented Jul 5, 2019

lmcbout commented Jul 8, 2019

lmcbout left a comment

akosyakov Jul 18, 2019 •

edited

Loading

fangnx Jul 18, 2019

paul-marechal Jul 19, 2019

akosyakov Jul 21, 2019 •

edited

Loading

fangnx Aug 1, 2019

akosyakov Aug 1, 2019

paul-marechal Aug 1, 2019

paul-marechal left a comment

Fixed #5454, #5452: modified "replace-all" to save changes without opening editors #5600

Fixed #5454, #5452: modified "replace-all" to save changes without opening editors #5600

Conversation

fangnx commented Jun 27, 2019 • edited Loading

vince-fugnitto left a comment

Choose a reason for hiding this comment

vince-fugnitto commented Jun 27, 2019 • edited Loading

vince-fugnitto commented Jun 27, 2019

lmcbout commented Jun 28, 2019

fangnx commented Jun 28, 2019

fangnx commented Jun 28, 2019

vince-fugnitto commented Jul 5, 2019

lmcbout commented Jul 8, 2019

lmcbout left a comment

Choose a reason for hiding this comment

akosyakov Jul 18, 2019 • edited Loading

Choose a reason for hiding this comment

fangnx Jul 18, 2019

Choose a reason for hiding this comment

paul-marechal Jul 19, 2019

Choose a reason for hiding this comment

akosyakov Jul 21, 2019 • edited Loading

Choose a reason for hiding this comment

fangnx Aug 1, 2019

Choose a reason for hiding this comment

akosyakov Aug 1, 2019

Choose a reason for hiding this comment

paul-marechal Aug 1, 2019

Choose a reason for hiding this comment

paul-marechal left a comment

Choose a reason for hiding this comment

fangnx commented Jun 27, 2019 •

edited

Loading

vince-fugnitto commented Jun 27, 2019 •

edited

Loading

akosyakov Jul 18, 2019 •

edited

Loading

akosyakov Jul 21, 2019 •

edited

Loading