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

Implement word part move and delete for issue #46203 #48023

Merged
merged 6 commits into from Jun 20, 2018

Conversation

Projects
None yet
6 participants
@brentwang23
Contributor

brentwang23 commented Apr 17, 2018

#46203

@msftclas

This comment has been minimized.

msftclas commented Apr 17, 2018

CLA assistant check
All CLA requirements met.

@brentwang23

This comment has been minimized.

Contributor

brentwang23 commented Apr 17, 2018

Hello all! I can see that CI is failing due to a formatting issue, but I haven't been able to find any information on how to fix it. Is there some sort of tool I can run to find my errors? Thanks!

@alexandrudima

This comment has been minimized.

Member

alexandrudima commented Apr 17, 2018

@brentwang23

File not formatted: src/vs/base/common/strings.ts
File not formatted: src/vs/editor/common/controller/cursorWordOperations.ts
File not formatted: src/vs/editor/contrib/wordPartOperations/test/wordPartOperations.test.ts

I wonder how you were able to commit since our pre-commit hooks should have failed. To fix, open these files in VSCode and run Format Document.

@brentwang23

This comment has been minimized.

Contributor

brentwang23 commented Apr 17, 2018

My apologies. After I couldn't figure out what the errors meant, I disabled the pre-commit check in the hopes someone would look at my pull request and help me find the problems. In hindsight, I probably should have just posted a question first. Thanks for the tip!

@alexandrudima alexandrudima added this to the June 2018 milestone Jun 20, 2018

@alexandrudima

This comment has been minimized.

Member

alexandrudima commented Jun 20, 2018

Thank you!

@alexandrudima alexandrudima merged commit 1cc8ec4 into Microsoft:master Jun 20, 2018

1 of 2 checks passed

VSTS: VS Code in progress
Details
license/cla All CLA requirements met.
Details
@benjaminjackman

This comment has been minimized.

ctrl+alt+delete may not be the best choice for a keyboard shortcut.

This comment has been minimized.

Member

alexandrudima replied Jul 3, 2018

:-)

@benjaminjackman

This comment has been minimized.

benjaminjackman commented Jul 3, 2018

So glad to see this being folded into vscode! After trying this out in insiders I have couple of minor comments:

Cursor location is non-symmetric for left versus right movements

This implementation puts the cursor in slightly difference places compared to sublime/intellij/the extension I have been using (https://github.com/ow--/vscode-subword-navigation)
Left and Right stop at different points between characters particularly with symbolic characters like '+'
e.g. ThisIs+A+Sample

in sublime/intellij/the extension they all stop at the locations marked with | regards of it the cursor is coming from the left or right:

|This|Is|+|A|+|Sample|

In this extension

if from the right
|This|Is|+A|+Sample|
and from the left
|This|Is+|A+|Sample|

Notice that the cursor will be in different asymmetric locations when going left or right.

Things get a bit more complicated in sublime vs intellij when there are multiple consecutive symbols, in sublime it appears as though the cursor will treat any block of symbols as one word (e.g.|++!+|) whereas in intellij it treats only the same consecutive symbol as a word (e.g. |++|!|+|)

Subword navigation doesn't appear to work in input box like the file-rename box in the file explorer

I could be mistaken on this but it looked like it wasn't working there when I tried it.

@kumarharsh

This comment has been minimized.

Contributor

kumarharsh commented Jul 3, 2018

+1 - it should work in the file input and other non-editor boxes, otherwise ow--'s plugin is good enough. I believe it was not possible in the plugin due to API limitations, which a native solution should be able to overcome.

@hesting15

This comment has been minimized.

hesting15 commented Jul 11, 2018

Not sure how it should work in this case:
TESTTestTest
but currently, it works like this:
T|E|S|T|Test|Test
Probably should be:
TEST|Test|Test

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