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 delete line with CRLF (#743) #770

Merged
merged 3 commits into from
Sep 15, 2016
Merged

Conversation

jgoz
Copy link
Contributor

@jgoz jgoz commented Sep 14, 2016

Fixes #743.

Delete was not taking a possible \r into consideration when performing a linewise delete, which resulted in an extra linebreak being appended to the line if it later was pasted.

Also removed a duplicate test related to this functionality ("Can copy to a register"). That test was failing on Windows with "files.eol": "crlf", but now passes after this change.

@xconverge
Copy link
Member

Why is the copy to a register test removed

@jgoz
Copy link
Contributor Author

jgoz commented Sep 15, 2016

@xconverge

Why is the copy to a register test removed

Per the PR description:

Also removed a duplicate test related to this functionality ("Can copy to a register"). That test was failing on Windows with "files.eol": "crlf", but now passes after this change.

As stated, the removed test was an exact duplicate of the one above it. The only reason I saw it is because the test was failing prior to my change.

@xconverge
Copy link
Member

Woops sorry totally missed that from my phone

I wonder why there was a duplicate..

@jgoz
Copy link
Contributor Author

jgoz commented Sep 15, 2016

No problem! Might be a merge conflict artifact.

Copy link
Member

@johnfn johnfn left a comment

Choose a reason for hiding this comment

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

Thanks! Just a minor change.

@@ -1062,7 +1062,8 @@ export class DeleteOperator extends BaseOperator {
let text = vscode.window.activeTextEditor.document.getText(new vscode.Range(start, end));

if (registerMode === RegisterMode.LineWise) {
text = text.slice(0, -1); // slice final newline in linewise mode - linewise put will add it back.
// slice final newline in linewise mode - linewise put will add it back.
text = text[text.length - 2] === '\r' ? text.slice(0, -2) : text.slice(0, -1);
Copy link
Member

@johnfn johnfn Sep 15, 2016

Choose a reason for hiding this comment

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

Can you ensure the line actually ends in \r\n?

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.

@johnfn
Copy link
Member

johnfn commented Sep 15, 2016

Excellent! Thank you, @jgoz!

@johnfn johnfn merged commit 002e682 into VSCodeVim:master Sep 15, 2016
@jgoz jgoz deleted the 743-fix-delete-crlf branch September 15, 2016 22:40
This pull request was closed.
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.

3 participants