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.

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

Find/Replace overlay: consistent handling of incremental base location (#1913 #1914) #1921

Conversation

Wittmaxi
Copy link

@Wittmaxi Wittmaxi commented Jun 3, 2024

Increment the incremental base location consistently, remove method for
"performIncrementalSearch" as it only called "performSearch" and lead to
semantic confusion while calling "performSearch".

  • "SearchOptions.REGEX" will now correctly update the incremental base
    location once it is disabled.
  • Added a method for updating the incremental base location manually
    from outside

fixes #1914
fixes #1913
fixes #1915

@Wittmaxi Wittmaxi force-pushed the MW_incremental_base_location branch from d3dadba to d9dd4b5 Compare June 3, 2024 12:07
Copy link
Contributor

github-actions bot commented Jun 3, 2024

Test Results

 1 814 files  +  604   1 814 suites  +604   1h 29m 22s ⏱️ + 22m 18s
 7 666 tests +    3   7 432 ✅ ±    0  233 💤 +  2  1 ❌ +1 
24 114 runs  +8 014  23 364 ✅ +7 777  749 💤 +236  1 ❌ +1 

For more details on these failures, see this check.

Results for commit c087338. ± Comparison against base commit ecfde49.

This pull request skips 5 and un-skips 3 tests.
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testEnabledWhenHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testMultipleHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testProblemHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testSingleHover
org.eclipse.ui.genericeditor.tests.ShowInformationTest ‑ testInformationControl
UiTestSuite org.eclipse.ui.tests.api.ApiTestSuite org.eclipse.ui.tests.api.WorkbenchPluginTest ‑ testGetImageRegistryFromAdditionalDisplay
org.eclipse.jface.text.tests.contentassist.ContextInformationTest ‑ testContextInfo_hide_focusOut
org.eclipse.urischeme.internal.registration.TestUnitWinRegistry ‑ testWinRegistry

♻️ This comment has been updated with latest results.

@Wittmaxi Wittmaxi force-pushed the MW_incremental_base_location branch 2 times, most recently from ff1ce9b to e4cf14d Compare June 10, 2024 07:38
Increment the incremental base location consistently, remove method for
"performIncrementalSearch" as it only called "performSearch" and lead to
semantic confusion while calling "performSearch".
- "SearchOptions.REGEX" will now correctly update the incremental base
location once it is disabled.
- Added a method for updating the incremental base location manually
  from outside

fixes eclipse-platform#1914
fixes eclipse-platform#1913
@HeikoKlare HeikoKlare force-pushed the MW_incremental_base_location branch from e4cf14d to c087338 Compare July 5, 2024 13:19
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

I have tested the changes and made two findings:

  1. Activating "selection in area" does not work properly anymore
    find_replace_selectinline

  2. When deactivating a search option that disables incremental search (in particular regex search) and the currently selected word still matches the search specification, still a new search is triggered, moving the selection to the next match. This behavior already existed before this PR but since this PR affects setting the incremental base position, it should probably be considered here.
    find_replace_disableregex

@HeikoKlare
Copy link
Contributor

Closing this as it has been split up into and replaced by #2104, #2106, #2108
Mentioned issues have also been addressed by those PRs.

@HeikoKlare HeikoKlare closed this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants