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

2024-03: "Find/Replace"-Button has no effect anymore without manually shifting focus back to the editor #1754

Closed
rhoehener opened this issue Mar 14, 2024 · 5 comments · Fixed by #1765
Closed

2024-03: "Find/Replace"-Button has no effect anymore without manually shifting focus back to the editor #1754

rhoehener opened this issue Mar 14, 2024 · 5 comments · Fixed by #1765
Labels
bug Something isn't working regression
Milestone
4.32

Comments

@rhoehener
Copy link

To reproduce:

  • Type Ctrl+F
  • Click the "Replace with"-field
  • Click the "Find/Replace"-button
  • Nothing happens until you click into the editor again

findreplace
@rhoehener rhoehener added the bug Something isn't working label Mar 14, 2024
@rhoehener rhoehener changed the title Eclipse 2024-03 regression: "Find/Replace"-Button has no effect anymore without manually shifting focus back to the editor 2024-03: "Find/Replace"-Button has no effect anymore without manually shifting focus back to the editor Mar 14, 2024
@iloveeclipse
Copy link
Member

I can reproduce in I20240313-1800.
The key point is to really click into both fields. If not clicking into "Find:" field, everything works.

@rhoehener : is this a regression? In which release it worked last?

@rhoehener
Copy link
Author

@iloveeclipse I noticed it immediately after switching from 2023-12 yesterday, so I'm assuming it's a regression since I use this functionality a lot, but not 100% sure.

I tested again: I can reproduce this reliably by immediately clicking the "Find/Replace"-button (no need to click the Find/Replace with fields).

@iloveeclipse iloveeclipse transferred this issue from eclipse-platform/eclipse.platform Mar 14, 2024
@iloveeclipse
Copy link
Member

iloveeclipse commented Mar 14, 2024 •
edited

OK, seems I can't reproduce on 4.30, so must be regression in 4.31.
The most likely candidate is #1132 or follow up work in #1503 and later changes.

@HeikoKlare , @Wittmaxi : could you please investigate?

@iloveeclipse iloveeclipse added this to the 4.32 milestone Mar 14, 2024
@HeikoKlare
Copy link
Contributor

Thanks for reporting the issue. We will have a look and provide a fix as soon as possible.

HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Mar 19, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog ec…
7c1d3cd 
…lipse-platform#1754

When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

This change performs the missing find operation before executing a
replace operation. It adds an according regression test.

Fixes eclipse-platform#1754
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Mar 19, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog ec…
02e8fda 
…lipse-platform#1754

When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

With this change, initializing the dialog with the current selection
performs the missing find operation before executing a replace
operation. The according functionality is refactored and a regression
test is added.

Fixes eclipse-platform#1754
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Mar 19, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog ec…
b12495d 
…lipse-platform#1754

When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

With this change, initializing the dialog with the current selection
performs the missing find operation before executing a replace
operation. The according functionality is refactored and a regression
test is added.

Fixes eclipse-platform#1754
@HeikoKlare HeikoKlare mentioned this issue Mar 19, 2024
Merged
@HeikoKlare
Copy link
Contributor

The reason for the bug is that replacement operations are enabled but no find operation has been executed before. An according exception is thrown and then swallowed by the dialog:

eclipse.platform.ui/bundles/org.eclipse.text/src/org/eclipse/jface/text/FindReplaceDocumentAdapter.java

Line 150 in 1122c63

throw new IllegalStateException("illegal findReplace state: cannot replace without preceding find"); //$NON-NLS-1$

Before #1132, some tracking for whether an initial search has to be performed before replace operations can be executed or before they should even be disabled was woven into several operations of the dialog class.

I've proposed a fix in #1765.

HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Mar 21, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog ec…
033648c 
…lipse-platform#1754

When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

With this change, initializing the dialog with the current selection
performs the missing find operation before executing a replace
operation. The according functionality is refactored and a regression
test is added.

Fixes eclipse-platform#1754
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Mar 23, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog ec…
f59d882 
…lipse-platform#1754

When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

With this change, initializing the dialog with the current selection
performs the missing find operation before executing a replace
operation. The according functionality is refactored and a regression
test is added.

Fixes eclipse-platform#1754
HeikoKlare added a commit that referenced this issue Mar 23, 2024
@HeikoKlare
Properly use selected string for replacements in FindReplaceDialog #1754
c40b7a1 


When opening the FindReplaceDialog with an active selection in a text
viewer, the input field for the find string is filled with the content
of that selection. In addition, the replace functionality of the dialog
is activated. Currently, using the replace functionality does not work
because it requires a find operation to be executed before, which is
currently not done.

With this change, initializing the dialog with the current selection
performs the missing find operation before executing a replace
operation. The according functionality is refactored and a regression
test is added.

Fixes #1754
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working regression
Projects
None yet
Milestone
4.32
3 participants
@HeikoKlare @iloveeclipse @rhoehener