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

Numbered, upper case and multicursor register #974

Merged
merged 11 commits into from
Oct 28, 2016

Conversation

Platzer
Copy link
Contributor

@Platzer Platzer commented Oct 24, 2016

Implements basic functionality for numbered registers (#500)

  • 0 for last yanked text
  • 1 to 9 delete history

Implements appending to register with uppercase letter

  • for single to single cursor register
  • for single to multi cursor register
  • for multi to multi cursor register (if line count match)

gif

Closes PR #918 because i was to stupid to rebase that branch...

@Platzer
Copy link
Contributor Author

Platzer commented Oct 24, 2016

@jpoon as you reviewed #918, looking forward the get your remarks on that one.

Copy link
Member

@jpoon jpoon left a comment

Choose a reason for hiding this comment

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

Sweet. Awesome work @Platzer. Sorry it took so long to give a review.

/**
* Puts the content at the specified index of the multicursor Register.
*
* `REMARKS:` This Procedure asume that you pass an valid register.
Copy link
Member

Choose a reason for hiding this comment

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

asume

spelling

Copy link
Member

Choose a reason for hiding this comment

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

Same spelling mistake in the other comments as well.

// Check if appending to a multicursor register or normal
if (typeof appendToRegister.text === "object") {
if (appendToRegister.registerMode === RegisterMode.CharacterWise && currentRegisterMode === RegisterMode.CharacterWise) {
(appendToRegister.text as string[]).forEach(element => {
Copy link
Member

Choose a reason for hiding this comment

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

Why not move this outside the if block as it's the same as line 218.

Register.registers[register].text = text;
}
// Harmonize newline character
text = text.replace(/\r\n/g, '\n');
Copy link
Member

Choose a reason for hiding this comment

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

Can we read files.eol to use the proper line ending of the file? https://code.visualstudio.com/Docs/customization/userandworkspace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These lines only apply when we read from the system clipboard. The problem is vscode automaticaly transforms \n to \r\n when your EOL is set to CRLF, so when we're on a windows machine the EOL is \r\n if i would paste that vscode transforms it to \r\r\n and you've got two linebreaks in the editor.
My first idea was to get the current EOL setting from the active editor but i didn't found the vscode API, you can set it but i don't found where i can read the setting???
As i can't go that way, my next idea was check the OS if we're on windows replace the newline char with the vscode-vim internal newline ('\n' we use it everywhere), but then i thought safe me the check and replace it everytime because on windows it fixes the double newline and on unix systems it causes no harm because there is no \r\n in the clipboard.

What do you think, is it better to add the OS check so it is more obvious that it should only apply to windows machines?

Copy link
Member

Choose a reason for hiding this comment

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

The following should work:

let eol = vscode.workspace.getConfiguration('files').get("eol", "\n");

and then you can add this as a new member to configuration.ts to expose it to your current functionality.

Copy link
Contributor Author

@Platzer Platzer Oct 27, 2016

Choose a reason for hiding this comment

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

So when i think about it the "The default end of line character" (comment for the setting) is not the problem, i would need the "current system clipboard end of line character" because i had to romve the \r only on windows because vscode adds it when \n is inserted. And in case of unix system replacing \n with \n is unnecessary.
See the behavior i get on my machine (WIN7 with EOL = CRLF):
vscodeinsertlinefeed
Every replace i do the \n is turned into \r\n so \n is the universial newline character in vscode?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, very interesting behaviour. I repro that on my mac and I see what you mean, in that case using \n is the way to go.

@jpoon jpoon merged commit eae04d8 into VSCodeVim:master Oct 28, 2016
@jpoon
Copy link
Member

jpoon commented Oct 28, 2016

Thanks @Platzer!

@Platzer Platzer deleted the register-rampage branch November 3, 2016 06:40
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.

2 participants