Skip to content

[Find/Replace dialog][Linux] "Replace" buttons remain disabled when using "Enter" #2882

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

iloveeclipse
Copy link
Member

Fixes #2880

Copy link
Contributor

github-actions bot commented Apr 1, 2025

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 32m 3s ⏱️ - 12m 1s
 7 918 tests ±0   7 690 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 841 runs  ±0  23 093 ✅ ±0  748 💤 ±0  0 ❌ ±0 

Results for commit 97630a6. ± Comparison against base commit cae93a3.

♻️ This comment has been updated with latest results.

@HeikoKlare
Copy link
Contributor

It might be that something has been mixed up in this handler by all the recent changes (including mine extracting the update of the button state from the handler). This change actually looks reasonable, but it will probably result in the same state of the code we had when this issue was reported:

And that issue was fixed by moving the modification handler around (in the opposite direction as done by this change):

I can have a look at that once I am back from travel (on Thursday). If someone wants to reevaluate this change with the above mentioned information, feel free to do so.

@iloveeclipse
Copy link
Member Author

Thanks Heiko, if I will have time tomorrow, I will check which workaround for which regression caused which problem.

@HeikoKlare
Copy link
Contributor

@iloveeclipse I have checked the implementation and the existing workaround again and this is my understanding how it has to behave:

  • the data passed to the underlying FindReplaceLogic (find and replace string) always have to updated when the modification listener is called
  • the button state must only be updated on the second call of the listener on Linux (that's the existing workaround)
    That's
    Since the listener is parameterized with both these kinds of operations, which have to be called under different conditions, they cannot be combined in a single handler as they are right now. One solution is to extend the listener parameterization to take two callbacks, one for each of the above mentioned situations.

I have just pushed according changes to the PR branch for demonstration purposes. In my opinion that should solve all mentioned issues. Feel free to adapt/revert/merge them as desired and required. I will leave for vacation now, so I will probably not be able to contribute further to this for the next week, but I hope the above mentioned helps to finalize the fix.

@iloveeclipse iloveeclipse merged commit 18b694c into eclipse-platform:master Apr 14, 2025
18 checks passed
@iloveeclipse iloveeclipse deleted the issue_2880 branch April 14, 2025 14:06
@iloveeclipse
Copy link
Member Author

@HeikoKlare : thanks, it works, I've verified that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Find/Replace dialog][Linux] "Replace" buttons remain disabled when using "Enter"
2 participants