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

[Fix] p in visual-mode should update register content #2602

Merged
merged 3 commits into from May 4, 2018

Conversation

tyru
Copy link
Contributor

@tyru tyru commented May 3, 2018

What this PR does / why we need it

This PR fixes p command to update register content, when it is executed in visual-mode.
Please see #1991 for the original Vim's behavior.

NOTE: this PR only fix to update register content.
p command is yet not Vim compatible behavior.
For example, the buffer content is:

hello
world

And cursor position is line 1, and type ddVpP results in:

world
hello

Unnecessary newline at the end.
Expected is:

world
hello

But this is another problem.
I will fix it later at the another PR.

Which issue(s) this PR fixes

Fixes #1991

@TravisBuddy
Copy link

Travis tests have failed

Hey @tyru,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

if [[ $(git diff-index HEAD --) ]]; then git diff; echo "Prettier Failed. Run `gulp` or `gulp forceprettier`"; exit 1; fi
diff --git a/src/actions/commands/actions.ts b/src/actions/commands/actions.ts
index 65eb072..f7fc06a 100644
--- a/src/actions/commands/actions.ts
+++ b/src/actions/commands/actions.ts
@@ -1555,7 +1555,11 @@ export class PutCommandVisual extends BaseCommand {
         true
       );
       const deletedRegister = await Register.getByKey(vimState.recordedState.registerName);
-      Register.putByKey(replaceRegister.text, vimState.recordedState.registerName, replaceRegister.registerMode);
+      Register.putByKey(
+        replaceRegister.text,
+        vimState.recordedState.registerName,
+        replaceRegister.registerMode
+      );
       // to ensure, that the put command nows this is
       // an linewise register insertion in visual mode of
       // characterwise, linewise
@@ -1563,7 +1567,11 @@ export class PutCommandVisual extends BaseCommand {
       deleteResult.currentMode = oldMode;
       deleteResult = await new PutCommand().exec(start, deleteResult, true);
       deleteResult.currentMode = resultMode;
-      Register.putByKey(deletedRegister.text, vimState.recordedState.registerName, deletedRegister.registerMode);
+      Register.putByKey(
+        deletedRegister.text,
+        vimState.recordedState.registerName,
+        deletedRegister.registerMode
+      );
       return deleteResult;
     }
 
Warning: The 'no-use-before-declare' rule requires type information.
Prettier Failed. Run [19:03:15] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[19:03:15] Starting 'prettier'...
[19:03:16] Starting 'tslint'...
[19:03:16] Starting 'compile'...
[19:03:19] Finished 'prettier' after 3.56 s
[19:03:22] Finished 'tslint' after 6.33 s
src/common/motion/position.ts(975,42): error TS2345: Argument of type 'number | ((start?: number | undefined, end?: number | undefined) => number[]) | ((...items: numbe...' is not assignable to parameter of type 'number'.
  Type '(start?: number | undefined, end?: number | undefined) => number[]' is not assignable to type 'number'.
TypeScript: 1 semantic error
TypeScript: emit succeeded (with errors)
[19:03:31] Finished 'compile' after 15 s
[19:03:31] Starting 'default'...
[19:03:31] Finished 'default' after 53 μs or [19:03:32] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[19:03:32] Starting 'forceprettier'...
[19:03:41] Finished 'forceprettier' after 9.42 s

@tyru tyru force-pushed the v_p-should-update-register branch from 4f9f1c1 to fb9d432 Compare May 3, 2018 19:07
title: 'Vp updates register content',
start: ['|hello', 'world'],
keysPressed: 'ddVpP',
end: ['|world', 'hello', ''],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment here mentioning that this test is actually not truly "correct"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done :)

@xconverge xconverge merged commit 54e181b into VSCodeVim:master May 4, 2018
@tyru tyru deleted the v_p-should-update-register branch May 4, 2018 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pasting in visual mode doesn't update the default buffer's contents
4 participants