Skip to content

Adds setting to disable cursor movement while typing into Find Widget#94825

Merged
rebornix merged 4 commits intomicrosoft:masterfrom
ultraGentle:master
Aug 11, 2020
Merged

Adds setting to disable cursor movement while typing into Find Widget#94825
rebornix merged 4 commits intomicrosoft:masterfrom
ultraGentle:master

Conversation

@ultraGentle
Copy link
Contributor

@ultraGentle ultraGentle commented Apr 9, 2020

This PR fixes #60977 and fixes #70306, both related to disorienting or disruptive cursor movement related to the Find Widget.

Description:

  • Adds a boolean setting to turn cursor movement on/off while typing into the Find Widget.
  • Default is true (on), which results in the current behavior.
  • False (off) results in highlighting-only on type, preventing the undesired/unexpected cursor movement described in 60977 and 70306. Jumping to the next match happens on Enter or on click the prev/next arrows in the widget.

Discussion:

  • The VSC team suggestion was to name the setting "searchOnType," however there is already a setting with that name. I chose "moveOnType."
  • Personally, I would reverse the defaults and simply note the option to switch back in "What's New," but I can understand not wanting to confuse existing users.
  • The VSC team suggested that the setting have three options: off | highlight | move. However, when I tested off, the behavior was just confusing, because it seemed like the Find Widget wasn't doing anything at all. Also, from searching related issues on GitHub, I haven't seen anyone asking for the "off" behavior. I thought it better not to overengineer a confusing 3rd setting that no one asked for.

Further improvements:

  • 70306 is only fixed when the new setting is activated (i.e. false). It could be addressed separately, but there is already a workaround discussed in that thread, and for anyone who doesn't want to do that, there is the option provided by this PR.
  • For this setting to be more easily discovered, I would suggest adding the words "jump" and "scroll" to the description, because those were what folks used when describing the issue (so that's probably what others would search for).

Any feedback is welcome, and I am happy for you to revise as you see fit.
Thanks for considering!

Copy link
Member

@rebornix rebornix left a comment

Choose a reason for hiding this comment

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

It seems the file is formatted by some formatters accidentally

@ultraGentle
Copy link
Contributor Author

ultraGentle commented Apr 9, 2020

Sorry about that.
Is there a command I can run to apply your in-house formatting automatically/retroactively?

EDIT: fixed.

@ultraGentle ultraGentle requested a review from rebornix April 10, 2020 14:11
@ultraGentle
Copy link
Contributor Author

@rebornix I believe I fixed the formatting issue. Any next steps you need from me?

@briankendall
Copy link

briankendall commented Apr 16, 2020

I just tried checking out and building this PR, and it does not fix #60977 or #70306 for me. Even with "search on type" disabled, when I click into another editor it will still jump to the first instance of whatever is in the find box. And typing into the box causes the editor to scroll to the first instance of whatever I'm typing. Changed the wrong setting.

@ultraGentle
Copy link
Contributor Author

ultraGentle commented Apr 16, 2020

@briankendall Can you verify that you un-selected move on type, and not search on type?

There is an unrelated, pre-existing setting search on type that has no effect.

The reason I ask is that @rebornix confirmed it worked in #60977 (comment) , and this PR is the same as my original, just behind a setting, so I thought maybe you just ticked the wrong one.

Maybe the naming could be clearer.

@briankendall
Copy link

@ultraGentle You're right, I changed the wrong setting! It is in fact working for me.

@ultraGentle
Copy link
Contributor Author

ultraGentle commented Apr 24, 2020

@rebornix I just made another commit:

  • Debounces typing into the Find Widget

  • This way, even users that do not disable moveOnType will avoid the editor auto-searching until they finish/pause typing -- which seems universally desirable.

  • If you just want the moveOnType setting without debounce, please merge the previous commit instead.

--
Edit: I note that the build failed on my latest commit; so maybe just use the previous one for now, and treat the "debounce" as a proof of concept.

As always, I'd appreciate any feedback on what I need to do to get this successfully merged. Thanks for considering!

@adamreisnz
Copy link

Any update on this getting merged?

@skplunkerin
Copy link

Same, I'd love to have this

@ultraGentle
Copy link
Contributor Author

I haven't heard from @rebornix that any changes are necessary, and as far as I can tell, all testers have had positive experiences with no issues. Hoping this will make it into next month's update!

@adamreisnz
Copy link

Ok that would be great, looking forward to it.

@ultraGentle
Copy link
Contributor Author

ultraGentle commented Jun 20, 2020

@adamreisnz , I think I implied that this will make it into the next release. If so, sorry -- that was just me hoping along with everyone else on this thread! I unfortunately don't have any insider info to share. I can't tell whether this is just pending, or has been dropped.

@dzoba
Copy link

dzoba commented Jul 17, 2020

Excited and hopeful to have this merged.

@ultraGentle
Copy link
Contributor Author

I'm not sure what the deal is with radio silence from the VSCode team regarding this issue. I've already pinged the reviewer, with no response, and I don't want to be a squeaky wheel, so it's not clear to me what the way forward is. If I knew there were a problem, I'd fix it, but without any feedback I'm flummoxed.

It's very strange to me, since this is a small PR and it fixes a super annoying bug, but I know folks are busy, so maybe there just hasn't been time for them to re-review and merge? Or maybe there's something else holding it up?

The opacity here is a disincentive to contribute to other issues, since it feels futile to work on something if it's not going to end up in the final product. Not complaining, promise, just confused!

@adamreisnz
Copy link

@rebornix would you mind reviewing this PR? It would really save us a lot of headaches with the cursor jumping to unexpected locations in the file.

@rebornix rebornix added this to the August 2020 milestone Aug 11, 2020
@rebornix rebornix merged commit c95626a into microsoft:master Aug 11, 2020
@rebornix
Copy link
Member

Thanks @ultraGentle for your contribution! I tweaked the code a bit and merged into master, should be available in Insiders later this week.

@ultraGentle
Copy link
Contributor Author

@rebornix My pleasure!

@TheInvoker
Copy link

Is this implemented? How can I enable this setting?

@ultraGentle
Copy link
Contributor Author

@TheInvoker Should be as of today or tomorrow. It's been in insiders edition for a month, so just waiting for 1.49 to be released. Setting is find > "moveCursorOnType" if I remember correctly.

@ultraGentle
Copy link
Contributor Author

We're finally there!

1.49.0 stable was released today with the setting
find > moveCursorOnType > set to false
which activates the desired behavior

Edit: Note that if you previously had global find clipboard set to false to avoid unexpected cursor jumping/highlighting when switching panes, you can now re-enable that setting, as moveCursorOnType > false fixes that bug.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editor jumps to random line when clicking into it Disable Auto Cursor Reveal while typing in Find Widget

7 participants