Skip to content

Support the "reason" arg for trimming trailing whitespace#35778

Merged
alexdima merged 3 commits intomicrosoft:masterfrom
dgileadi:trim-whitespace-preserve-cursor
Oct 24, 2017
Merged

Support the "reason" arg for trimming trailing whitespace#35778
alexdima merged 3 commits intomicrosoft:masterfrom
dgileadi:trim-whitespace-preserve-cursor

Conversation

@dgileadi
Copy link
Contributor

@dgileadi dgileadi commented Oct 7, 2017

This is in support of #24400. The reason this enhancement is needed is that extensions like editorconfig-vscode call the editor.action.trimTrailingWhitespace command during an auto-save event, and without knowing whether the command was triggered by an auto-save event the code can't know whether to trim trailing whitespace at the cursor (causing the cursor to move) or not.

This change will allow fixing editorconfig-vscode #47.

@msftclas
Copy link

msftclas commented Oct 7, 2017

CLA assistant check
All CLA requirements met.

private preserveCursor: boolean;

constructor(selection: Selection) {
constructor(selection: Selection, preserveCursor?: boolean) {
Copy link

Choose a reason for hiding this comment

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

Shouldn't it be preserving multiple cursors? Not just one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I woke up realizing that passing a boolean was silly—I should just provide the cursors directly to the command. Fixed that.


public getEditOperations(model: editorCommon.ITokenizedModel, builder: editorCommon.IEditOperationBuilder): void {
let ops = trimTrailingWhitespace(model, []);
var cursors: [Position];
Copy link

Choose a reason for hiding this comment

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

const cursors = (this.selection && this.preserveCursor)
    ? [new Position(this.selection.positionLineNumber, this.selection.positionColumn)]
    : [Position]
;

private preserveCursor: boolean;

constructor(selection: Selection) {
constructor(selection: Selection, preserveCursor?: boolean) {
Copy link

Choose a reason for hiding this comment

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

Consider preserveCursor = false here for a default value (makes it conditional).

@dgileadi
Copy link
Contributor Author

Hi @alexandrudima! Is there any thing I can do to help get this in?

@alexdima
Copy link
Member

@dgileadi Sorry I'm a bit slow... I was "offline" for a bit to work on #36410

@alexdima alexdima added this to the October 2017 milestone Oct 24, 2017
@alexdima
Copy link
Member

Thank you @dgileadi for your contribution ! ❤️
Thank you @jedmao for reviewing ❤️

@alexdima alexdima merged commit dd5e66e into microsoft:master Oct 24, 2017
@dgileadi dgileadi deleted the trim-whitespace-preserve-cursor branch October 24, 2017 16:43
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants