Skip to content

Conversation

@magnushiie
Copy link
Contributor

Also add test cases (2nd test case with comma is currently commented out)

Partially fixes #14675

Also add test cases (2nd test case with comma is currently commented out)

Partially fixes microsoft#14675
@magnushiie
Copy link
Contributor Author

It's a bit hard to understand the indentation architecture, and this change might be the wrong place to fix it. Specifically, it's hard to understand how indentation and delta apply to multi-line nodes and the individual tokens belonging to them.

@magnushiie
Copy link
Contributor Author

The behavior in smartIndentOnUnclosedFunctionDeclaration04 baffles me, have to study it some more. The obviously incorrect fix to that test would be for the last 3 lines:

-verifyIndentationAfterNewLine("4", 8);
-verifyIndentationAfterNewLine("5", 4);
-verifyIndentationAfterNewLine("6", 4);
+verifyIndentationAfterNewLine("4", 12);
+verifyIndentationAfterNewLine("5", 8);^
+verifyIndentationAfterNewLine("6", 8);

On the other hand, if you type in the example in #14675, it's clear the smart indentation and re-formatting are out of sync already today.

@zhengbli
Copy link
Contributor

The fix is pretty trickey. The issue is that previously we consider two cases:

  1. the child node starts at the same line with the parent node;
    In this case, the indentation should be
  • if that line was not indented already -> the same as the parent's indentation;
  • if that line was indented -> the indentation of that line
  1. the child node does not start at the same line with the parent node:
    In this case, the indentation should be the same as the parent's indentation + delta;

Consider a similar case with the issue at hand:

Promise
    .then(foo => {
        console.log(foo);
    })

Here the indentation of function block is the same as the .then line, because the block node starts at the same line with the parent node (the arrow expression node), then it inherits indentation from that line.

However in our case,

Promise
    .then(
        foo
    )

The foo node does not starts from the same line with its parent node (the entire Promise.then ... call expression node), so it does not use the indentation from that line, which caused the issue.

I'm thinking about a more comprehensive way to fix this.

@zhengbli
Copy link
Contributor

Hi I submitted a new PR at #14955
Can you help check if it solves the issue? Thanks

@magnushiie
Copy link
Contributor Author

Closing this in favor of #14955

@magnushiie magnushiie closed this Apr 3, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 21, 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.

Incorrect formatting in fluent API call

3 participants