Skip to content

Enable code formatter to enforce consistent whitespace style #489

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

Merged
merged 3 commits into from
Feb 6, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,29 @@
"type": "boolean",
"default": true,
"description": "A new line must follow an open brace."
},
"powershell.codeFormatting.whitespaceBeforeOpenBrace": {
"type": "boolean",
"default": true,
"description": "There must be a whitespace between a keyword and its associated scriptblock expression."
},
"powershell.codeFormatting.whitespaceBeforeOpenParen": {
"type": "boolean",
"default": true,
"description": "There must be whitespace between an keyword (if, elseif, while, switch, etc) and its associated conditional expression."
},
"powershell.codeFormatting.whitespaceAroundOperator": {
"type": "boolean",
"default": true,
"description": "There must be whitespaces around both sides of a binary or assignment operator ('=', '+', '-', etc.)."
},
"powershell.codeFormatting.whitespaceAfterSeparator": {
"type": "boolean",
"default": true,
"description": "There must be a whitespaces after a separator (',' and ';')."
}
}
}
},
"private": true
}
}
15 changes: 12 additions & 3 deletions src/features/DocumentFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ class PSDocumentFormattingEditProvider implements DocumentFormattingEditProvider
private readonly ruleOrder: string[] = [
"PSPlaceCloseBrace",
"PSPlaceOpenBrace",
"PSUseWhitespace",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be named PSUseConsistentWhitespace?

Copy link
Author

Choose a reason for hiding this comment

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

I was in a bit of a limbo on whether to have "consistent" in its name. But looks like "PSUseConsistentWhitespace" sounds more consistent with the "PSUseConsistentIndentation" rule. On the other hand all the rules enforce some or the other kind of consistency and so having the "consistent" word seemed verbose (And I was even thinking of changing "PSUseConsistentIndentation" to "PSUseIndentation", but that would be a breaking change for PSSA). At this point, I personally don't have any preference but if you think "PSUseConsistentWhitespace" sounds better, I can change it to that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think consistency would be nice between the two names. "PSUseIndentation/Whitespace" doesn't sound specific enough to me; the "Consistent" designation helps with that. I've tried thinking of other names and I can't come up with anything better at the moment.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed!

Also made corresponding changes to PowerShell/PSScriptAnalyzer#702

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks man!

"PSUseConsistentIndentation"];

// Allows edits to be undone and redone is a single step.
Expand Down Expand Up @@ -232,12 +233,13 @@ class PSDocumentFormattingEditProvider implements DocumentFormattingEditProvider
});

// We cannot handle multiple edits at the same point hence we
// filter the markers so that there is only one edit per line
// This ideally should not happen but it is good to have some additional safeguard
// filter the markers so that there is only one edit per region
if (edits.length > 0) {
uniqueEdits.push(edits[0]);
for (let edit of edits.slice(1)) {
if (editComparer(uniqueEdits[uniqueEdits.length - 1], edit) !== 0) {
let lastEdit: ScriptRegion = uniqueEdits[uniqueEdits.length - 1];
if (lastEdit.startLineNumber !== edit.startLineNumber
|| (edit.startColumnNumber + edit.text.length) < lastEdit.startColumnNumber) {
uniqueEdits.push(edit);
}
}
Expand Down Expand Up @@ -332,6 +334,13 @@ class PSDocumentFormattingEditProvider implements DocumentFormattingEditProvider
ruleSettings["IndentationSize"] = vscode.workspace.getConfiguration("editor").get<number>("tabSize");
break;

case "PSUseWhitespace":
ruleSettings["CheckOpenBrace"] = psSettings.codeFormatting.whitespaceBeforeOpenBrace;
ruleSettings["CheckOpenParen"] = psSettings.codeFormatting.whitespaceBeforeOpenParen;
ruleSettings["CheckOperator"] = psSettings.codeFormatting.whitespaceAroundOperator;
ruleSettings["CheckSeparator"] = psSettings.codeFormatting.whitespaceAfterSeparator;
break;

default:
break;
}
Expand Down
10 changes: 9 additions & 1 deletion src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import vscode = require('vscode');
export interface ICodeFormattingSettings {
openBraceOnSameLine: boolean;
newLineAfterOpenBrace: boolean;
whitespaceBeforeOpenBrace: boolean;
whitespaceBeforeOpenParen: boolean;
whitespaceAroundOperator: boolean;
whitespaceAfterSeparator: boolean;
}

export interface IScriptAnalysisSettings {
Expand Down Expand Up @@ -50,7 +54,11 @@ export function load(myPluginId: string): ISettings {

let defaultCodeFormattingSettings: ICodeFormattingSettings = {
openBraceOnSameLine: true,
newLineAfterOpenBrace: true
newLineAfterOpenBrace: true,
whitespaceBeforeOpenBrace: true,
whitespaceBeforeOpenParen: true,
whitespaceAroundOperator: true,
whitespaceAfterSeparator: true
};

return {
Expand Down