Support keeping results in results of `Find in Files` and `Replacing in Files` #104676

jinyangcruise · 2025-03-27T02:09:08Z

Closes godotengine/godot-proposals#11923

Support keeping multiple searching/replacing results.

before	after

arkology · 2025-03-27T04:11:34Z

I feel like all these 3 text buttons in future could be replaced with just 3 icons.
Lock/refresh/close is really common and easy recognizable actions to represent as icons.

lodetrick · 2025-06-11T08:16:21Z

editor/find_in_files.cpp

+const char *FindInFilesTab::SIGNAL_RESULT_SELECTED = "result_selected";
+const char *FindInFilesTab::SIGNAL_FILES_MODIFIED = "files_modified";
+const char *FindInFilesTab::SIGNAL_CLOSE_BUTTON_CLICKED = "close_button_clicked";


I don't think its best to store the signal names as constants, this is not something I've seen anywhere else in the codebase. However, if you do end up keeping these, they should be StringName instead of char *.

I basically imitated the defining way of signal names in class FindInFiles, class FindInFilesDialog, class FindInFilesPanel in the way of writing. You can check find_in_files.h. Indeed I don't know why they wrote it that way. I can modify it as you said. Do you think I should also modify those lines in class FindInFiles, class FindInFilesDialog and class FindInFilesPanel?

This entire file seems inconsistent with the rest of the editor, even the ordering of the private, protected, public is different from what I've seen in other places

Well, I think it's better that I only modify the codes that I wrote😅Other modifications can be put on a separate pull request.

lodetrick · 2025-06-11T08:17:43Z

editor/find_in_files.cpp

+void FindInFilesPanel::set_search_labels_visibility(bool p_visible) {
+	_find_label->set_visible(p_visible);
+	_search_text_label->set_visible(p_visible);
+}


The Close button is also redundant in the TabContainer view, consider also hiding that.

lodetrick · 2025-06-11T08:24:10Z

editor/find_in_files.h

+class TabContainer;
+
+// Contains several panels
+class FindInFilesTab : public TabContainer {


There is an extra margin around the TabContainer (even when the tabs are hidden), look at editor_debugger_node for how they solved this same issue. Essentially make this class inherit from MarginContainer and set your margins to negative the panel's margins. Although this will probably need a change in EditorBottomPanel. I've solved this problem in a separate PR that won't be merged for a while, so I'll create a small bugfix PR that fixes this specifically in an easier to review form.

Edit: Included in #107395

So I don't need to take care of this part, right?

The PR that I've linked fixed a bug in the TabContainer theming where the corner radius is too large. You would still need to do the same fix that the EditorDebuggerNode does, but using the BottomPanel stylebox instead of the BottomPanelDebuggerOverride stylebox.

godot/editor/debugger/editor_debugger_node.cpp

Lines 68 to 75 in 51b0379

add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));

add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));

add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));

tabs = memnew(TabContainer);

tabs->set_tabs_visible(false);

tabs->connect("tab_changed", callable_mp(this, &EditorDebuggerNode::_debugger_changed));

add_child(tabs);

Above is an excerpt that should give you an idea of what I'm saying. Just so you're aware, what I'm referring to as the margin is the empty space above, below, and to the side of the TabContainer. This is especially visible on the top as the space above the tabs but is equally there on the sides and bottom.

Thx, I fixed this part.

lodetrick · 2025-06-11T08:35:26Z

editor/find_in_files.cpp

+	bool bar_visible = get_tab_count() > 1;
+	set_tabs_visible(bar_visible);
+
+	// Panel's some labels' texts are duplicate with tab's title, so we want to change their visibility depends on the bar's visibility


Suggested change

// Panel's some labels' texts are duplicate with tab's title, so we want to change their visibility depends on the bar's visibility

// Panel's some labels' texts are duplicate with tab's title, so we want to change their visibility depends on the bar's visibility.

lodetrick · 2025-06-11T08:41:31Z

editor/find_in_files.h

+class PopupMenu;
+class TabContainer;
+
+// Contains several panels


Suggested change

// Contains several panels

// Contains several panels.

Also would it be possible to make this more specific?

Done. More details added.

lodetrick · 2025-06-12T02:19:00Z

editor/find_in_files.cpp

+	add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));
+	add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));
+	add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));


Suggested change

add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));

add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));

add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));

add_theme_constant_override("margin_top", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_TOP));

add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));

add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));

add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));

This also needs to be added to NOTIFICATION_THEME_CHANGED so when the theme changes, the margins are able to update

Yes. BTW, SNAME("BottomPanelDebuggerOverride") should be changed to SNAME("BottomPanel")?

Yes, because this dock does not have the BottomPanelDebuggerOverride stylebox as its background, instead it uses BottomPanel as its stylebox, and so uses its margins.

lodetrick · 2025-06-12T07:29:21Z

editor/find_in_files.h

+	FindInFilesPanel *create_new_panel();
+	FindInFilesPanel *get_curr_panel();


These two can be made private, and you don't need to abbreviate current to curr, it only hurts readability.

Thanks, fixed.

lodetrick · 2025-06-12T07:46:25Z

editor/find_in_files.h

 	Button *_replace_all_button = nullptr;
 };
+
+class MarginContainer;


Suggested change

class MarginContainer;

You don't need to forward declare this class

lodetrick · 2025-06-12T07:47:49Z

editor/find_in_files.h


 	void set_with_replace(bool with_replace);
 	void set_replace_text(const String &text);
+	bool keep_results() const;


Maybe make the function name more descriptive like will_keep_results or is_keeping_results, to make it clear that it is a getter function.

Thanks, fixed.

lodetrick

Beyond some small things, it looks good! A note for future reviewers, the artifact in the top right is fixed in a separate PR (#107395):

lodetrick · 2025-06-12T08:11:57Z

editor/find_in_files.h

+	FindInFilesPanel *create_new_panel();
+	FindInFilesPanel *get_current_panel();


Suggested change

FindInFilesPanel *create_new_panel();

FindInFilesPanel *get_current_panel();

FindInFilesPanel *_create_new_panel();

FindInFilesPanel *_get_current_panel();

Private functions should have the "_" prefix

Thanks, I also fixed private variable tabs to _tabs. Appreciate your earnestness.

jinyangcruise requested a review from a team as a code owner March 27, 2025 02:09

jinyangcruise force-pushed the find_in_files_keep_results branch 2 times, most recently from 96bb797 to aaaafd8 Compare March 27, 2025 08:37

AThousandShips added enhancement topic:editor labels Mar 27, 2025

AThousandShips added this to the 4.x milestone Mar 27, 2025

jinyangcruise force-pushed the find_in_files_keep_results branch 2 times, most recently from 84af7d4 to 7e718a8 Compare March 28, 2025 05:40

jinyangcruise force-pushed the find_in_files_keep_results branch from 7e718a8 to 38888c1 Compare June 11, 2025 07:25

lodetrick reviewed Jun 11, 2025

View reviewed changes

lodetrick mentioned this pull request Jun 11, 2025

Fix TabContainer Editor theming and remove Debugger style hacks #107395

Merged

jinyangcruise force-pushed the find_in_files_keep_results branch 2 times, most recently from 64b3b16 to c0c5fd9 Compare June 12, 2025 01:59

lodetrick reviewed Jun 12, 2025

View reviewed changes

jinyangcruise force-pushed the find_in_files_keep_results branch from c0c5fd9 to c4178be Compare June 12, 2025 03:29

jinyangcruise requested a review from lodetrick June 12, 2025 05:28

lodetrick reviewed Jun 12, 2025

View reviewed changes

jinyangcruise force-pushed the find_in_files_keep_results branch from c4178be to 9685de0 Compare June 12, 2025 08:06

lodetrick approved these changes Jun 12, 2025

View reviewed changes

jinyangcruise force-pushed the find_in_files_keep_results branch from 9685de0 to 1c456bd Compare June 12, 2025 08:31

arkology mentioned this pull request Jun 12, 2025

[Editor] Use Icons for FindInFiles Buttons #107431

Open

jinyangcruise force-pushed the find_in_files_keep_results branch 2 times, most recently from 05407d3 to 53ddcb4 Compare June 24, 2025 06:39

jinyangcruise force-pushed the find_in_files_keep_results branch from 53ddcb4 to 1565d54 Compare July 18, 2025 03:47

jinyangcruise force-pushed the find_in_files_keep_results branch 2 times, most recently from e52e35d to 8793058 Compare September 19, 2025 10:43

jinyangcruise force-pushed the find_in_files_keep_results branch from 8793058 to 3f00759 Compare October 31, 2025 03:58

Support Keep Results in results of Find in Files

04dfd7a

jinyangcruise force-pushed the find_in_files_keep_results branch from 3f00759 to 04dfd7a Compare November 3, 2025 03:14

	add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));
	add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));
	add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));

	tabs = memnew(TabContainer);
	tabs->set_tabs_visible(false);
	tabs->connect("tab_changed", callable_mp(this, &EditorDebuggerNode::_debugger_changed));
	add_child(tabs);

	// Panel's some labels' texts are duplicate with tab's title, so we want to change their visibility depends on the bar's visibility
	// Panel's some labels' texts are duplicate with tab's title, so we want to change their visibility depends on the bar's visibility.

-	add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));
-	add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));
-	add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanelDebuggerOverride"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));
+	add_theme_constant_override("margin_top", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_TOP));
+	add_theme_constant_override("margin_left", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_LEFT));
+	add_theme_constant_override("margin_right", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_RIGHT));
+	add_theme_constant_override("margin_bottom", -EditorNode::get_singleton()->get_editor_theme()->get_stylebox(SNAME("BottomPanel"), EditorStringName(EditorStyles))->get_margin(SIDE_BOTTOM));

		FindInFilesPanel *create_new_panel();
		FindInFilesPanel *get_curr_panel();

		FindInFilesPanel *create_new_panel();
		FindInFilesPanel *get_current_panel();

Uh oh!

Support keeping results in results of Find in Files and Replacing in Files #104676

Are you sure you want to change the base?

Support keeping results in results of Find in Files and Replacing in Files #104676

Conversation

jinyangcruise commented Mar 27, 2025

Uh oh!

arkology commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lodetrick Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lodetrick Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lodetrick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Support keeping results in results of `Find in Files` and `Replacing in Files` #104676

Support keeping results in results of `Find in Files` and `Replacing in Files` #104676

arkology commented Mar 27, 2025 •

edited

Loading

lodetrick Jun 11, 2025 •

edited

Loading

lodetrick Jun 11, 2025 •

edited

Loading