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.

Sign up for GitHub

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 formatting on JsxElement/JsxExpression #9361

Merged
merged 7 commits into from
Jun 28, 2016
Merged

Fix formatting on JsxElement/JsxExpression #9361

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c021ed5
Merge remote-tracking branch 'refs/remotes/Microsoft/master'
saschanaz Mar 30, 2016
03e38a5
Merge https://github.com/Microsoft/TypeScript
saschanaz Apr 11, 2016
6e415cc
Merge branch 'master' of https://github.com/Microsoft/TypeScript
saschanaz Jun 25, 2016
9dfcb44
do not format comma/closeparen in jsxelement
saschanaz Jun 25, 2016
378c6b5
format jsx expression
saschanaz Jun 26, 2016
cbfbfe1
make rules optional
saschanaz Jun 27, 2016
6e984eb
new options should be optional for compatibility
saschanaz Jun 28, 2016
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
1 change: 1 addition & 0 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ namespace FourSlash {
InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: false,
InsertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets: false,
InsertSpaceAfterOpeningAndBeforeClosingTemplateStringBraces: false,
InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces: false,
PlaceOpenBraceOnNewLineForFunctions: false,
PlaceOpenBraceOnNewLineForControlBlocks: false,
};
Expand Down
1 change: 1 addition & 0 deletions src/server/editorServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1580,6 +1580,7 @@ namespace ts.server {
InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: false,
InsertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets: false,
InsertSpaceAfterOpeningAndBeforeClosingTemplateStringBraces: false,
InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces: false,
PlaceOpenBraceOnNewLineForFunctions: false,
PlaceOpenBraceOnNewLineForControlBlocks: false,
});
Expand Down
26 changes: 23 additions & 3 deletions src/services/formatting/rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ namespace ts.formatting {
public NoSpaceBeforeTemplateMiddleAndTail: Rule;
public SpaceBeforeTemplateMiddleAndTail: Rule;

// No space after { and before } in JSX expression
public NoSpaceAfterOpenBraceInJsxExpression: Rule;
public SpaceAfterOpenBraceInJsxExpression: Rule;
public NoSpaceBeforeCloseBraceInJsxExpression: Rule;
public SpaceBeforeCloseBraceInJsxExpression: Rule;

constructor() {
///
/// Common Rules
Expand Down Expand Up @@ -316,7 +322,7 @@ namespace ts.formatting {

// Add a space between statements. All keywords except (do,else,case) has open/close parens after them.
// So, we have a rule to add a space for [),Any], [do,Any], [else,Any], and [case,Any]
this.SpaceBetweenStatements = new Rule(RuleDescriptor.create4(Shared.TokenRange.FromTokens([SyntaxKind.CloseParenToken, SyntaxKind.DoKeyword, SyntaxKind.ElseKeyword, SyntaxKind.CaseKeyword]), Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsNotForContext), RuleAction.Space));
this.SpaceBetweenStatements = new Rule(RuleDescriptor.create4(Shared.TokenRange.FromTokens([SyntaxKind.CloseParenToken, SyntaxKind.DoKeyword, SyntaxKind.ElseKeyword, SyntaxKind.CaseKeyword]), Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isNonJsxElementContext, Rules.IsNotForContext), RuleAction.Space));

// This low-pri rule takes care of "try {" and "finally {" in case the rule SpaceBeforeOpenBraceInControl didn't execute on FormatOnEnter.
this.SpaceAfterTryFinally = new Rule(RuleDescriptor.create2(Shared.TokenRange.FromTokens([SyntaxKind.TryKeyword, SyntaxKind.FinallyKeyword]), SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Space));
Expand Down Expand Up @@ -444,8 +450,8 @@ namespace ts.formatting {
///

// Insert space after comma delimiter
this.SpaceAfterComma = new Rule(RuleDescriptor.create3(SyntaxKind.CommaToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsNextTokenNotCloseBracket), RuleAction.Space));
this.NoSpaceAfterComma = new Rule(RuleDescriptor.create3(SyntaxKind.CommaToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete));
this.SpaceAfterComma = new Rule(RuleDescriptor.create3(SyntaxKind.CommaToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isNonJsxElementContext, Rules.IsNextTokenNotCloseBracket), RuleAction.Space));
this.NoSpaceAfterComma = new Rule(RuleDescriptor.create3(SyntaxKind.CommaToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isNonJsxElementContext), RuleAction.Delete));

// Insert space before and after binary operators
this.SpaceBeforeBinaryOperator = new Rule(RuleDescriptor.create4(Shared.TokenRange.Any, Shared.TokenRange.BinaryOperators), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsBinaryOpContext), RuleAction.Space));
Expand Down Expand Up @@ -491,6 +497,12 @@ namespace ts.formatting {
this.NoSpaceBeforeTemplateMiddleAndTail = new Rule(RuleDescriptor.create4(Shared.TokenRange.Any, Shared.TokenRange.FromTokens([SyntaxKind.TemplateMiddle, SyntaxKind.TemplateTail])), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete));
this.SpaceBeforeTemplateMiddleAndTail = new Rule(RuleDescriptor.create4(Shared.TokenRange.Any, Shared.TokenRange.FromTokens([SyntaxKind.TemplateMiddle, SyntaxKind.TemplateTail])), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Space));

// No space after { and before } in JSX expression
this.NoSpaceAfterOpenBraceInJsxExpression = new Rule(RuleDescriptor.create3(SyntaxKind.OpenBraceToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isJsxExpressionContext), RuleAction.Delete));
this.SpaceAfterOpenBraceInJsxExpression = new Rule(RuleDescriptor.create3(SyntaxKind.OpenBraceToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isJsxExpressionContext), RuleAction.Space));
this.NoSpaceBeforeCloseBraceInJsxExpression = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.CloseBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isJsxExpressionContext), RuleAction.Delete));
this.SpaceBeforeCloseBraceInJsxExpression = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.CloseBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.isJsxExpressionContext), RuleAction.Space));

// Insert space after function keyword for anonymous functions
this.SpaceAfterAnonymousFunctionKeyword = new Rule(RuleDescriptor.create1(SyntaxKind.FunctionKeyword, SyntaxKind.OpenParenToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext), RuleAction.Space));
this.NoSpaceAfterAnonymousFunctionKeyword = new Rule(RuleDescriptor.create1(SyntaxKind.FunctionKeyword, SyntaxKind.OpenParenToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext), RuleAction.Delete));
Expand Down Expand Up @@ -723,6 +735,14 @@ namespace ts.formatting {
return context.TokensAreOnSameLine() && context.contextNode.kind !== SyntaxKind.JsxText;
}

static isNonJsxElementContext(context: FormattingContext): boolean {
return context.contextNode.kind !== SyntaxKind.JsxElement;
}

static isJsxExpressionContext(context: FormattingContext): boolean {
return context.contextNode.kind === SyntaxKind.JsxExpression;
}

static IsNotBeforeBlockInFunctionDeclarationContext(context: FormattingContext): boolean {
return !Rules.IsFunctionDeclContext(context) && !Rules.IsBeforeBlockContext(context);
}
Expand Down
9 changes: 9 additions & 0 deletions src/services/formatting/rulesProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@ namespace ts.formatting {
rules.push(this.globalRules.NoSpaceBeforeTemplateMiddleAndTail);
}

if (options.InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces) {
rules.push(this.globalRules.SpaceAfterOpenBraceInJsxExpression);
rules.push(this.globalRules.SpaceBeforeCloseBraceInJsxExpression);
}
else {
rules.push(this.globalRules.NoSpaceAfterOpenBraceInJsxExpression);
rules.push(this.globalRules.NoSpaceBeforeCloseBraceInJsxExpression);
}

if (options.InsertSpaceAfterSemicolonInForStatements) {
rules.push(this.globalRules.SpaceAfterSemicolonInFor);
}
Expand Down
1 change: 1 addition & 0 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,7 @@ namespace ts {
InsertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: boolean;
InsertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets: boolean;
InsertSpaceAfterOpeningAndBeforeClosingTemplateStringBraces: boolean;
InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be optional, or it is a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but I'm not sure what you mean by 'optional', do you want me to make it insert?: boolean form?

PlaceOpenBraceOnNewLineForFunctions: boolean;
PlaceOpenBraceOnNewLineForControlBlocks: boolean;
[s: string]: boolean | number | string | undefined;
Expand Down
32 changes: 26 additions & 6 deletions tests/cases/fourslash/formattingJsxElements.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//// </div>
//// )
////}
////
////
////function foo1() {
//// return (
//// <div className="commentBox" data-id="test">
Expand Down Expand Up @@ -45,8 +45,8 @@
//// class3= {/*5*/
//// }/>/*6*/
//// )
////}
////
////}
////
////(function () {
//// return <div
////className=""/*attrAutoformat*/
Expand All @@ -64,7 +64,14 @@
/////*childJsxElementIndent*/
////<span></span>/*grandchildJsxElementAutoformat*/
////</span>/*containedClosingTagAutoformat*/
////</h5>
////</h5>;
////
////<div>,{integer}</div>;/*commaInJsxElement*/
////<div>, {integer}</div>;/*commaInJsxElement2*/
////<span>)</span>;/*closingParenInJsxElement*/
////<span>) </span>;/*closingParenInJsxElement2*/
////<Router routes={ 3 } />;/*jsxExpressionSpaces*/
////<Router routes={ (3) } />;/*jsxExpressionSpaces2*/

format.document();
goTo.marker("autoformat");
Expand Down Expand Up @@ -114,7 +121,7 @@ verify.indentationIs(12);

goTo.marker("danglingBracketAutoformat")
// TODO: verify.currentLineContentIs(" >");
verify.currentLineContentIs(" >");
verify.currentLineContentIs(" >");
goTo.marker("closingTagAutoformat");
verify.currentLineContentIs(" </div>");

Expand All @@ -125,4 +132,17 @@ verify.indentationIs(8);
goTo.marker("grandchildJsxElementAutoformat");
verify.currentLineContentIs(" <span></span>");
goTo.marker("containedClosingTagAutoformat");
verify.currentLineContentIs(" </span>");
verify.currentLineContentIs(" </span>");

goTo.marker("commaInJsxElement");
verify.currentLineContentIs("<div>,{integer}</div>;");
goTo.marker("commaInJsxElement2");
verify.currentLineContentIs("<div>, {integer}</div>;");
goTo.marker("closingParenInJsxElement");
verify.currentLineContentIs("<span>)</span>;");
goTo.marker("closingParenInJsxElement2");
verify.currentLineContentIs("<span>) </span>;");
goTo.marker("jsxExpressionSpaces");
verify.currentLineContentIs("<Router routes={3} />;");
goTo.marker("jsxExpressionSpaces2");
verify.currentLineContentIs("<Router routes={(3)} />;");
32 changes: 32 additions & 0 deletions tests/cases/fourslash/formattingOptionsChangeJsx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
///<reference path="fourslash.ts"/>

//@Filename: file.tsx
/////*InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces*/<Madoka homu={ true } saya={ (true) } />;

runTest("InsertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces", "<Madoka homu={ true } saya={ (true) } />;", "<Madoka homu={true} saya={(true)} />;");


function runTest(propertyName: string, expectedStringWhenTrue: string, expectedStringWhenFalse: string) {
// Go to the correct file
goTo.marker(propertyName);

// Set the option to false first
format.setOption(propertyName, false);

// Format
format.document();

// Verify
goTo.marker(propertyName);
verify.currentLineContentIs(expectedStringWhenFalse);

// Set the option to true
format.setOption(propertyName, true);

// Format
format.document();

// Verify
goTo.marker(propertyName);
verify.currentLineContentIs(expectedStringWhenTrue);
}