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 multicursor snippet placeholder directions #8423

Merged
merged 1 commit into from
Sep 30, 2023

Conversation

pascalkuthe
Copy link
Member

Closes #8422

The code worked as intended for the most part, the head < anchor and anchor < head case was just swapped accidentally. This caused a selection to be created from the first tabstop to the anchor (if the selection is forward) which would be merged with all other tabstop selections.

@pascalkuthe pascalkuthe added E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer. C-bug Category: This is a bug A-language-server Area: Language server client labels Sep 29, 2023
@perlinm
Copy link

perlinm commented Sep 29, 2023

Confirming that I tested this locally and it fixes #8422! Thank you!

Minor addendum: with these changes, whether I move the cursor after entering insert-mode no longer affects the cursor location(s), but it still affects what text is selected when I add the snippet. Any ideas why that might be and how to fix it? Not a huge deal if this isn't resolved 🙂

@pascalkuthe
Copy link
Member Author

that is on purpose, we only move the selection head and not the anchor. for example if you use |\be| selected and then enter insert mode with a and select the completion you should endup with

|\begin{|}

\end{||}

This matches what would happen if you manually type same if the cursor is in the reverse direction in insertmode.

Movement does change the cursor direction (to forward) outside of select mode

@archseer archseer merged commit 4e86d1c into master Sep 30, 2023
6 checks passed
@archseer archseer deleted the placeholder_direction branch September 30, 2023 03:28
danillos pushed a commit to danillos/helix that referenced this pull request Nov 21, 2023
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LSP behavior depends on selection history
3 participants