Screen lines and characters. #486
Conversation
rebornix
changed the title
| if (vimState.currentMode === ModeName.Normal) { | ||
| return Position.FromVSCodePosition(vscode.window.activeTextEditor.selection.active); | ||
| } else { | ||
| /** |
There was a problem hiding this comment.
This is a big bummer; it will slow down macros and . considerably.
I know that it's not your fault at all, but any additional griping you can do in the direction of VSCode would be helpful 馃槃
There was a problem hiding this comment.
I agree it's a bummer when users use this command quite often. Do we have any benchmark (or you have ideas about how) about commands to help measure how bad it is?
If it turns out to be unacceptablely slow, I'd like to push it hard from Code side.
There was a problem hiding this comment.
Hmm.
Upon reflection, a mild slowdown here is acceptable, because not too many people use these commands (they are a bit esoteric). At this point, I think that a history API is priority number 1, and that we can safely bump the priority of getting actual positions down. I just hope that the people over at MS don't mark this as finished and forget about it.
If you can push just for one thing, I would really like the history API. We are really suffering a lot because of our reimplementation of that.
There was a problem hiding this comment.
Let me make History the 1st feature request for next sprint.
|
When you fix up my minor comments this LGTM. I would like to hold off until vscode 1.4 ships with the new features before merging this however, as I suspect a lot of our users are not using the insiders version, and we want to support them as well. |
|
@johnfn fixed per your comments. Actually you don't need to wait for 1.4 as these commands have no side effect in any version below, that's saying even if you are using an old Code, when you run these new Code commands, Code will just eat them and emit nothing. This is also a good way to test our new features unlocked by Code. People who use Insider can try these new features first and may report issues if there is any bug. The key point is, making sure newly added features have no side effect and are backwards compatible. |
|
Thanks @rebornix! |
Yay! We love PRs! 馃帄
Please include a description of your change & check your PR against this list, thanks:
gulp tslint)We are adding APIs for screen lines/characters in Insider and this PR is integrating those APIs for following commands
g0g^g$gmgkgjHMLAs @johnfn mentioned while I did #478, the way we implment Movement in VSCodeVim is calculating the target Position or Selection and update the View in a central place (modelHandler). We are likely doing this from the very beginning.
While we are exposing commands in VS Code, we try to avoid exposing View Model information as they are way too complex and always require additional work. Take
scroll to next screen lineas example, we want VSCodeVim just provides the Position information and handle the movment ourselves, but for other extension authors or just users who want to customize key bindings, they have to leverage other APIs to move the cursor to target position.So in this PR, I'm calling VS Code API to do the real movment or selection and return the correct Position/Movement information after the command is executed.