Fix formatting of parameter list of a multiple-line call expression #14955
Conversation
zhengbli
force-pushed
the
fixFormatCallExpression
branch
from
df2da92 to
adeb9a6
Compare
|
This works great, thank you! |
|
One additional (somewhat related, but I understand this is PropertyAccessExpression vs ElementAccessExpression, not about CallExpression) question, shouldn't the following be formatted the same way? Promise
['then'](
cb
);
Promise
['then'](
cb,
); |
|
Yes that does make sense, I'll try to cover this case and update later. Thanks for pointing out! |
| case SyntaxKind.CallExpression: | ||
| return (<CallExpression>parent).arguments === nodes; | ||
| default: | ||
| return false; |
There was a problem hiding this comment.
nit: would merge this with the if clause above.
| * Here the parent of argument list is the call expression itself, however the indentation of the argument list should carry | ||
| * over the indentation in line 1 (".then"). | ||
| */ | ||
| function shouldListInheritIndentationFromLastIndentedLine(listStartLine: number, parent: Node, nodes: NodeArray<Node>) { |
There was a problem hiding this comment.
Could you comment with an above example what is the value of "listStartLine"?
| inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, startLine, /*isListItem*/ true, /*isFirstListItem*/ i === 0); | ||
| } | ||
|
|
||
| if (nodes.hasTrailingComma && formattingScanner.isOnToken()) { |
There was a problem hiding this comment.
Is this clause is so that we will indent trailing comma appropriately?
There was a problem hiding this comment.
Yes. The current issue is the formatter does not expect a trailing comma; it checks after all children nodes if the the current token is not the expected list ending token, it will do nothing. And then the list end token will be formatted later as part of the parent node using the parent's dynamic indentations. It worked fine before most of the time because normally the list uses the same indentation with its parent node; however, in this particular case they are different, therefore the trailing comma is causing problems. This change is to ensure that the list end token is processed with the list not its parent.
|
Sorry to bother you, but I think this is kind of duplicated work of #13574? |
|
@saschanaz Hmm I was not aware of your PR. I'll take a look and if it covers my fix I'll close this. Thanks. |
|
Is this or #13574 going to be merged some time? |
|
Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it. |
8 participants
Fixes #14675