FindReplaceOverlay: use search-as-you-type in regex mode #1911 #2655
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
Contributor
Test Results 1 821 files ±0 1 821 suites ±0 1h 27m 19s ⏱️ - 1m 45s Results for commit 8bd6fc0. ± Comparison against base commit 36c6b93. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
HeikoKlare
force-pushed
the
findreplace-incremental-with-regex
branch
from
2970bb6 to
020d00d
Compare
HeikoKlare
changed the title
Contributor
Author
|
@jukzi may I ask you to test whether this change fixes the behavior/issue you reported in #2066 (comment) for performing text replacements in regex mode? |
Contributor
|
I was reading this yesterday and I wondered if this is really such a great idea. With more complex regular expressions you often end up in an intermediate state with syntax errors. Moreover if I type |
Contributor
|
Too much magic is also unintuitive. Disabling search-as-you-type for regexp may be unwanted magic (especially if the search string is a plain text). Not searching for invalid regexp is expectable. reverting to a previous position would probably too much magic, the use can still manually repositon manually. I tested this PR with: Still it does not work to search "main" (+ explicitly using search forward) and then replacing with |
Contributor
Author
Yes, that's definitely a drawback I see as well. The current search match may be "jumping around" while you try to correct your expression. On the other hand, the benefit of this is that you directly see where a regex matches while typing, so you have an "example match" that directly allows you to have some validation of correctness of your expression. Note that other tools / IDEs (VS Code, IntelliJ) work like this as well: they have search-as-you-type always enabled, even in regex mode. I would currently expect the benefits for the user (consistent behavior, direct feedback) and developer (reduced complexity) to outweigh potential drawbacks or "needs to adapt" to the changed behavior.
For anyone interested, here is a video capturing the behavior:
Why is that "totally unintuitive"? I would say that's exactly the expected behavior.
Well, this is mixing up different things. Replacing with |
Contributor
|
testing this PR i found this Error in my Log: |
Contributor
|
beside the error i think it's intuitive to have search as you type enabled for regexp as provided by this PR. |
Contributor
Author
|
Thank you! The error is unrelated to this PR. I have never seen it before but it is related to disablement/enablement of key bindings for the FindReplaceOverlay. At first impression, it rather seems to be some issue in the according functionality of the text editor, but since that one is not "official" API and only used via reflection (just like the breadcrumb does it), there is of course no guarantee to work under every condition. Definitely something to take a look into. If no one objects to the PR itself (enabling search-as-you-type for regex mode), I would like to give it a try next week. I will then address this as a follow-up: |
HeikoKlare
force-pushed
the
findreplace-incremental-with-regex
branch
from
11cf37b to
af5f0db
Compare
Closed
2 tasks
Contributor
Author
|
Thank you again for reporting the error, Jörg! I can reproduce it and have created a dedicated issue for it as it is unrelated to this PR: I have created a separate PR proposing a fix for it. |
…form#1911 Currently, the FindReplaceLogic disables the incremental mode (i.e., search-as-you-type) in case the regex search option is activated. This makes the FindReplaceOverlay, which uses search-as-you-type in all other situations, behave non-intuitively when the regex option is activated. In order to achieve consistent, user-expectable behavior of the find and replace functionality and because of lacking arguments against using search-as-you-type in regex mode, this change makes the regex search mode independent from the incremental mode of the FindReplaceLogic. In consequence, the FindReplaceOverlay always uses search-as-you-type, no matter which search options are activated. In addition, explicit test code for that exceptional behavior is removed or adapted. Contributes to eclipse-platform#2066 Contributes to eclipse-platform#2646 Fixes eclipse-platform#1911
HeikoKlare
force-pushed
the
findreplace-incremental-with-regex
branch
from
af5f0db to
8bd6fc0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the FindReplaceLogic disables the incremental mode (i.e., search-as-you-type) in case the regex search option is activated. This makes the FindReplaceOverlay, which uses search-as-you-type in all other situations, behave non-intuitively when the regex option is activated.
In order to achieve consistent, user-expectable behavior of the find and replace functionality and because of lacking arguments against using search-as-you-type in regex mode, this change makes the regex search mode independent from the incremental mode of the FindReplaceLogic. In consequence, the FindReplaceOverlay always uses search-as-you-type, no matter which search options are activated.
In addition, explicit test code for that exceptional behavior is removed or adapted.
Contributes to #2066
Contributes to #2646
Fixes #1911
Note that this also aligns the behavior with similar functionality in other tools (like VS code).