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

Standardize transpose logic. Add basic support for transposing multibyte characters. #45499

Merged

Conversation

JacksonKearl
Copy link
Contributor

This fixes #8574 by standardizing "editor.action.transposeLetters" to match standard emacs/MacOS behavior.

@JacksonKearl
Copy link
Contributor Author

Additionally fixes some strange behavior whereby transposing multibyte characters either deletes them or splits them, corrupting the document. This behavior still exists in some edge cases, not too sure why.

@alexdima
Copy link
Member

👍 Thank you!

@alexdima alexdima merged commit 0f2274f into microsoft:master Mar 26, 2018
@alexdima alexdima added this to the March 2018 milestone Mar 26, 2018
@Zarel
Copy link
Contributor

Zarel commented Mar 27, 2018

While developing my transpose extension and comparing existing implementations, I noticed differences between the macOS and emacs implementation.

Specifically, in:

A
B

If you have the cursor immediately before B:

A
|B

^t will do nothing in macOS, but in emacs, it will swap the preceding and next characters:

AB|
[empty line]

I don't know if you care about fixing corner cases, but this might be a useful thing to be aware of, at least.

@JacksonKearl
Copy link
Contributor Author

JacksonKearl commented Mar 27, 2018

@Zarel when was this? In current macOS, the behaviour is as you described for emacs, except the cursor remains on the same line:

A
|B

becomes:

AB
|

This, to me, is the most logical because the typical (non-endline) behaviour of transpose is "swap char before with char after, then move cursor after the swapped pair". In this case char before is newline, char after is B, so \nB becomes B\n, and we put the cursor after the newline.

In fact, now that I look at it, emacs puts the cursor on the next line as well, so I think all is well.

With this PR, vscode exactly mirrors TextEdit (except in weird multi-byte corner cases that I mostly but not entirely removed), which I believe also means it exactly mirrors emacs (except emacs doesn't seem to handle multi-byte at all).

Nice extension BTW, "splitIntoLines" seems great. Might have to take it.

@Zarel
Copy link
Contributor

Zarel commented Mar 27, 2018

Oh, huh, yeah, you're right.

I was testing in Chrome's <textarea> widget in macOS, but in hindsight I should probably have tested in a native macOS text field like TextEdit.

@JacksonKearl JacksonKearl deleted the feature-request/standard-transpose branch March 28, 2018 00:53
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 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.

Ctrl-t (transpose) command inserts extra character.
3 participants