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
merged 1 commit into from Mar 26, 2018

Conversation

Projects
None yet
3 participants
@JacksonKearl
Copy link
Contributor

JacksonKearl commented Mar 11, 2018

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

@JacksonKearl

This comment has been minimized.

Copy link
Contributor

JacksonKearl commented Mar 11, 2018

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.

@alexandrudima

This comment has been minimized.

Copy link
Member

alexandrudima commented Mar 26, 2018

👍 Thank you!

@alexandrudima alexandrudima merged commit 0f2274f into Microsoft:master Mar 26, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@alexandrudima alexandrudima added this to the March 2018 milestone Mar 26, 2018

@Zarel

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

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 JacksonKearl:feature-request/standard-transpose branch Mar 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment