Skip to content

Properly indent JSXText on format document#19619

Merged
uniqueiniquity merged 5 commits into
microsoft:masterfrom
uniqueiniquity:indentJsxText
Nov 1, 2017
Merged

Properly indent JSXText on format document#19619
uniqueiniquity merged 5 commits into
microsoft:masterfrom
uniqueiniquity:indentJsxText

Conversation

@uniqueiniquity
Copy link
Copy Markdown
Contributor

Currently, only the first line of a contiguous block of text within a JSX element properly respects indentation when the document is formatted; all others are unaffected. This change inserts whitespace as needed so that all other lines indent properly as well.

@uniqueiniquity uniqueiniquity requested review from a user and RyanCavanaugh October 31, 2017 21:16
//// )
////}

goTo.position(21);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use a marker instead

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Or just use verify.currentFileContentIs instead of textAtCaretIs.

Comment thread src/services/formatting/formatting.ts Outdated
processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);

if (child.kind === SyntaxKind.JsxText) {
const range = { pos: child.getStart(), end: child.getEnd() };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Object literal has no contextual type, this can lead to wierd errors. Use const range: TextRange or just include it inline.

verify.textAtCaretIs("hello");
goTo.position(50);
verify.textAtCaretIs("goodbye");
goTo.marker('0');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this whole test would be simpler with verify.currentFileContentIs -- and you could just remove everything except the <div> from the file.

@uniqueiniquity uniqueiniquity merged commit dcc1f14 into microsoft:master Nov 1, 2017
@uniqueiniquity uniqueiniquity deleted the indentJsxText branch November 1, 2017 16:03
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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.

3 participants