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: VSCodeNotifyVisual and VSCodeCallVisual produce incorrect VSCode selections in visual and visual block modes if the cursor is placed at the beginning of the visual selection #1180

Conversation

archilkarchava
Copy link
Contributor

Fixes VSCodeNotifyVisual and VSCodeCallVisual producing incorrect VSCode selections in visual and visual block modes when the cursor is placed at the beginning of the visual selection.

vim.keymap.set("x", "<D-d>, "<Cmd>call VSCodeNotifyVisual('editor.action.addSelectionToNextFindMatch', 1)<CR><Esc>i", opts) mapping was used in the examples below.
But the problem exists for all mappings that call either VSCodeNotifyVisual or VSCodeCallVisual.

Before

Screen.Recording.2023-03-18.at.16.41.11.mp4

After

Screen.Recording.2023-03-18.at.16.38.41.mp4

… selections in visual and visual block modes if the cursor is placed at the beginning of the visual selection
Copy link
Collaborator

@justinmk justinmk left a comment

Choose a reason for hiding this comment

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

Nice, thanks.

Comment on lines 62 to 66
if (startPos[1] == endPos[1] && startPos[2] > endPos[2]) || startPos[1] > endPos[1]
let startPos[2] = startPos[2] + 1
else
let endPos[2] = endPos[2] + 1
endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

since this is used in multiple places, it's worth extracting it to a function like fixVisualPos(). That gives a hint to future readers that happen to look at this function. Also helps if a bug fix is needed.

Is it possible to add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justinmk, I've extracted duplicated logic into the s:fixVisualPos function and added a test.

@justinmk justinmk merged commit f9bcd25 into vscode-neovim:master Apr 23, 2023
4 checks passed
theol0403 added a commit to theol0403/vscode-neovim that referenced this pull request Jun 27, 2023
chore(master): release 0.1.0

BEGIN_COMMIT_OVERRIDE
feat: Neovim toggle (vscode-neovim#1033)
fix: wrong VSCode selections if cursor is at start of selection vscode-neovim#1180
feat: add $NVIM_APPNAME option (vscode-neovim#1186)
feat: use k instead of i to moveEditorToAboveGroup (vscode-neovim#1119)
feat(ci): add automatic releases (and switch to semantic versioning)
END_COMMIT_OVERRIDE
theol0403 added a commit to theol0403/vscode-neovim that referenced this pull request Jun 27, 2023
BEGIN_COMMIT_OVERRIDE
feat: Neovim toggle (vscode-neovim#1033)
fix: wrong VSCode selections if cursor is at start of selection vscode-neovim#1180
feat: add $NVIM_APPNAME option (vscode-neovim#1186)
feat: use k instead of i to moveEditorToAboveGroup (vscode-neovim#1119)
feat(ci): add automatic releases (and switch to semantic versioning)
END_COMMIT_OVERRIDE
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

2 participants