From 00f69b287ce22d4e0bfc45444c8147bb578f27bf Mon Sep 17 00:00:00 2001 From: hetmankp Date: Thu, 3 Oct 2019 13:09:53 +1000 Subject: [PATCH 1/2] Fixes gj/gk behaviour on wrapped lines; closes #4120 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This more or less reverts the functionality introduced by commit 4b2e07933 and raised in issue #3890. Unfortunately it is not currently possible to fully implement display line movements over wrapped lines that will always preserve cursor position. One of two things is necessary to change this, either the VSCode API needs to be extended to return information about wrapped lines (but this was rejected in issue microsoft/vscode#23045). Alternatively VSCodeVim would need to reïimplement all motions using vscode.commands.executeCommand() commands rather than manipulating vscode.window.activeTextEditor.selections directly, as the former preserves column position while the latter does not. However it's not clear to me if all VIM beahviour could be reïmplemented this way without first trying this out and sounds like a bit of a ground up rewrite. It seems for now we're stuck with gj/gk not preserving column position when jumping across short lines. --- src/actions/motion.ts | 17 +++++++++++++++-- test/mode/modeVisual.test.ts | 14 -------------- test/mode/normalModeTests/motions.test.ts | 14 -------------- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/actions/motion.ts b/src/actions/motion.ts index f984dcf105c..facc32a7029 100644 --- a/src/actions/motion.ts +++ b/src/actions/motion.ts @@ -163,6 +163,19 @@ abstract class MoveByScreenLineMaintainDesiredColumn extends MoveByScreenLine { } } +class MoveDownByScreenLineMaintainDesiredColumn extends MoveByScreenLineMaintainDesiredColumn { + movementType: CursorMovePosition = 'down'; + by: CursorMoveByUnit = 'wrappedLine'; + value = 1; +} + + +class MoveUpByScreenLineMaintainDesiredColumn extends MoveByScreenLineMaintainDesiredColumn { + movementType: CursorMovePosition = 'up'; + by: CursorMoveByUnit = 'wrappedLine'; + value = 1; +} + class MoveDownFoldFix extends MoveByScreenLineMaintainDesiredColumn { movementType: CursorMovePosition = 'down'; by: CursorMoveByUnit = 'line'; @@ -785,7 +798,7 @@ class MoveScreenLineCenter extends MoveByScreenLine { } @RegisterAction -export class MoveUpByScreenLineMaintainDesiredColumn extends MoveByScreenLineMaintainDesiredColumn { +export class MoveUpByDisplayLine extends MoveByScreenLine { modes = [ModeName.Insert, ModeName.Normal, ModeName.Visual]; keys = [['g', 'k'], ['g', '']]; movementType: CursorMovePosition = 'up'; @@ -794,7 +807,7 @@ export class MoveUpByScreenLineMaintainDesiredColumn extends MoveByScreenLineMai } @RegisterAction -class MoveDownByScreenLineMaintainDesiredColumn extends MoveByScreenLineMaintainDesiredColumn { +class MoveDownByDisplayLine extends MoveByScreenLine { modes = [ModeName.Insert, ModeName.Normal, ModeName.Visual]; keys = [['g', 'j'], ['g', '']]; movementType: CursorMovePosition = 'down'; diff --git a/test/mode/modeVisual.test.ts b/test/mode/modeVisual.test.ts index 13f5e88ff5d..8c9c119741e 100644 --- a/test/mode/modeVisual.test.ts +++ b/test/mode/modeVisual.test.ts @@ -240,20 +240,6 @@ suite('Mode Visual', () => { keysPressed: 'vgjx', end: ['blah', 'duh', '|ur'], }); - - newTest({ - title: "Preserves cursor position when handling 'gk'", - start: ['blah', 'word', 'a', 'la|st'], - keysPressed: 'vgkgkx', - end: ['blah', 'wo|t'], - }); - - newTest({ - title: "Preserves cursor position when handling 'gj'", - start: ['blah', 'wo|rd', 'a', 'last'], - keysPressed: 'vgjgjx', - end: ['blah', 'wo|t'], - }); }); suite('handles aw in visual mode', () => { diff --git a/test/mode/normalModeTests/motions.test.ts b/test/mode/normalModeTests/motions.test.ts index 5dbc4894847..92de1ba1836 100644 --- a/test/mode/normalModeTests/motions.test.ts +++ b/test/mode/normalModeTests/motions.test.ts @@ -739,18 +739,4 @@ suite('Motions in Normal Mode', () => { keysPressed: 'gj', end: ['blah', 'duh', 'dur', '|hur'], }); - - newTest({ - title: "Preserves cursor position when handling 'gk'", - start: ['blah', 'duh', 'a', 'hu|r '], - keysPressed: 'gkgk', - end: ['blah', 'du|h', 'a', 'hur '], - }); - - newTest({ - title: "Preserves cursor position when handling 'gj'", - start: ['blah', 'du|h', 'a', 'hur '], - keysPressed: 'gjgj', - end: ['blah', 'duh', 'a', 'hu|r '], - }); }); From 7a0e18b85b758447168c3b8a6b7cd9fc8ee3e224 Mon Sep 17 00:00:00 2001 From: hetmankp Date: Fri, 4 Oct 2019 14:44:04 +1000 Subject: [PATCH 2/2] Add gj/gk tests for future behaviour These tests had been previously removed as supporting this behaviour is not yet possible due to technical reasons, however they do represent correct VIM behaviour so we keep them until implementation becomes architecturally possible. --- test/mode/modeVisual.test.ts | 14 ++++++++++++++ test/mode/normalModeTests/motions.test.ts | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test/mode/modeVisual.test.ts b/test/mode/modeVisual.test.ts index 9455cd2b29c..58f8aafb896 100644 --- a/test/mode/modeVisual.test.ts +++ b/test/mode/modeVisual.test.ts @@ -247,6 +247,20 @@ suite('Mode Visual', () => { keysPressed: 'vgjx', end: ['blah', 'duh', '|ur'], }); + + newTestSkip({ + title: "Preserves cursor position when handling 'gk'", + start: ['blah', 'word', 'a', 'la|st'], + keysPressed: 'vgkgkx', + end: ['blah', 'wo|t'], + }); + + newTestSkip({ + title: "Preserves cursor position when handling 'gj'", + start: ['blah', 'wo|rd', 'a', 'last'], + keysPressed: 'vgjgjx', + end: ['blah', 'wo|t'], + }); }); suite('handles aw in visual mode', () => { diff --git a/test/mode/normalModeTests/motions.test.ts b/test/mode/normalModeTests/motions.test.ts index 05ee677cb15..6dcd0d478a0 100644 --- a/test/mode/normalModeTests/motions.test.ts +++ b/test/mode/normalModeTests/motions.test.ts @@ -739,4 +739,18 @@ suite('Motions in Normal Mode', () => { keysPressed: 'gj', end: ['blah', 'duh', 'dur', '|hur'], }); + + newTestSkip({ + title: "Preserves cursor position when handling 'gk'", + start: ['blah', 'duh', 'a', 'hu|r '], + keysPressed: 'gkgk', + end: ['blah', 'du|h', 'a', 'hur '], + }); + + newTestSkip({ + title: "Preserves cursor position when handling 'gj'", + start: ['blah', 'du|h', 'a', 'hur '], + keysPressed: 'gjgj', + end: ['blah', 'duh', 'a', 'hu|r '], + }); });