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: Cursor offset #2511

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Conversation

fredizzimo
Copy link
Member

@fredizzimo fredizzimo commented Apr 28, 2024

What kind of change does this PR introduce?

Did this PR introduce a breaking change?

  • No

Use EditorState struct instead of passing parameters around
Copy link

Test Results

  6 files  ±0    6 suites  ±0   19s ⏱️ +4s
110 tests ±0  110 ✅ ±0  0 💤 ±0  0 ❌ ±0 
644 runs  ±0  644 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 2f184f1. ± Comparison against base commit a5a9484.

@9mm
Copy link
Contributor

9mm commented Apr 28, 2024

I checked on this branch and it still occurs, but now its only 1 line off instead of 2

@fredizzimo
Copy link
Member Author

Hm strange, I just tested all top paddings from 0-20, and all of them work. Maybe this is some macOS specific issue? Maybe affected by the --frame parameter?

@9mm
Copy link
Contributor

9mm commented Apr 28, 2024

hmm.. i have spent tons of hours coding today so my mind is tired, but as a quick test i tried...

neovide git:(pr/2511) NEOVIDE_FRAME=full NEOVIDE_TITLE_HIDDEN=0 cargo run

which i think are the defaults and i got the same effect

these rae the current ones i use

export NEOVIDE_FRAME=buttonless
export NEOVIDE_TITLE_HIDDEN=1
export NEOVIDE_VSYNC=0
export NEOVIDE_FORK=1

@9mm
Copy link
Contributor

9mm commented Apr 28, 2024

i will eat some food and poke around

@9mm
Copy link
Contributor

9mm commented Apr 29, 2024

I tried a bunch of different options and couldnt seem to make it budge ;/ any ideas of what i should check?

@fredizzimo
Copy link
Member Author

Hm.. this PR is supposed to fix the case with the padding. It's broken on main for me and this fixes it.

But I just noticed another bug, the offset gets wrong with splits.

@9mm
Copy link
Contributor

9mm commented Apr 29, 2024

ughhh sorry about that, I was on main with the padding test. im confused now (just woke up)

what i said originally is true though .. it does definitely occur on this branch as that much i have tested a bunch of times

@fredizzimo
Copy link
Member Author

Ah, that gave me a clue. If I open a tabbar, then the offset gets wrong in the vertical direction.

@9mm
Copy link
Contributor

9mm commented Apr 29, 2024

ah! yes I use a tabbar

@fredizzimo
Copy link
Member Author

Ok, I will fix that, once I find the exact cause in the code.

I'm sorry about this, I should have tested my changes better. I think I did when I originally wrote it, but there was so many code changes and refactoring after that, which probably broke it.

@9mm
Copy link
Contributor

9mm commented Apr 29, 2024

Don't apologize you are the GOAT! You along with core team make everyones editing experience infinitely better and for that Im thankful every time I open Neovide.

@fredizzimo
Copy link
Member Author

@9mm, I think it should be fixed now.

@9mm
Copy link
Contributor

9mm commented Apr 29, 2024

works perfect now!

@fredizzimo fredizzimo changed the title fix: Cursor offset with padding fix: Cursor offset Apr 29, 2024
@fredizzimo fredizzimo merged commit 5efb713 into neovide:main Apr 29, 2024
11 checks passed
@fredizzimo
Copy link
Member Author

I went ahead and merged this, since it's a pretty bad regression.

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.

Cursor is offset from click
2 participants