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

Fix mouseshape to be set correctly when using 'r' or 'gr' #12157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ychin
Copy link
Contributor

@ychin ychin commented Mar 15, 2023

Currently, when entering the "pretend" or single character replace modes using r or gr (in GUI), the mouse cursor doesn't immediately update until you have re-focused the window or moved the mouse. This is because it's not calling update_mouseshape(-1) immediately, so the cursor will only be updated when it's called by other functions like gui_mouse_focus (which happens just if you move the mouse or re-focus the application).

To fix this, just make sure we call this update_mouseshape(-1). It's what we do when entering Insert or Replace modes for example.

I noticed this when trying to figure out why MacVim CI is failing in Test_mouse_shape_after_cancelling_gr (introduced in #12110), but I think that test is only passing in Vim GTK CI by accident, since this issue happens there too. I think the window captured focus after the mouse gr call which triggers a mouse shape change but it probably would have failed under other circumstances.

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #12157 (bea9ff8) into master (1433802) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #12157      +/-   ##
==========================================
- Coverage   81.95%   81.94%   -0.01%     
==========================================
  Files         160      164       +4     
  Lines      193810   194091     +281     
  Branches    43840    43829      -11     
==========================================
+ Hits       158834   159052     +218     
- Misses      22138    22207      +69     
+ Partials    12838    12832       -6     
Flag Coverage Δ
huge-clang-none 82.66% <100.00%> (-0.01%) ⬇️
huge-gcc-none 53.84% <0.00%> (+<0.01%) ⬆️
huge-gcc-testgui 51.96% <0.00%> (-0.01%) ⬇️
huge-gcc-unittests 0.29% <0.00%> (?)
linux 82.38% <100.00%> (-0.02%) ⬇️
mingw-x64-HUGE 76.54% <100.00%> (-0.01%) ⬇️
mingw-x86-HUGE 77.00% <100.00%> (-0.01%) ⬇️
windows 78.13% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/normal.c 90.99% <100.00%> (+<0.01%) ⬆️

... and 34 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ychin ychin force-pushed the fix-mouseshape-single-replace-mode branch from 78af162 to 620f5b8 Compare March 16, 2023 00:37
Currently, when entering the "pretend" or single character replace modes
using `r` or `gr`, the mouse cursor doesn't immediately update until you
have re-focused the window or moved the mouse. This is because it's not
calling `update_mouseshape(-1)` immediately, so the cursor will only be
updated when it's called by other functions like `gui_mouse_focus`.

To fix this, just make sure we call this `update_mouseshape(-1)`. It's
what we do when entering Insert or Replace modes for example.

I noticed this when trying to figure out why MacVim CI is failing in
`Test_mouse_shape_after_cancelling_gr` (introduced in vim#12110), but I
think that test is only passing in Vim GTK CI by accident, since this
issue happens there too. I think the window captured focus after the
mouse `gr` call which triggers a mouse shape change but it probably
would have failed under other circumstances.
@brammool
Copy link
Contributor

brammool commented Apr 16, 2023 via email

@ychin
Copy link
Contributor Author

ychin commented Apr 17, 2023

Sure. Let me take a look.

@yegappan
Copy link
Member

Any updates on this?

@yegappan yegappan added this to the backlog milestone Aug 13, 2023
@ychin
Copy link
Contributor Author

ychin commented Aug 14, 2023

I remember the tests were a little tricky to actually write to make it a proper regression test, and then other things came up and I neglected to follow up on this. Let me take a look again.

@chrisbra
Copy link
Member

any updates here please ?

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.

None yet

4 participants