Skip to content

fix: formatting for chaining methods#21027

Merged
1 commit merged into
microsoft:masterfrom
alan-agius4:feature/format-chains
Jan 16, 2018
Merged

fix: formatting for chaining methods#21027
1 commit merged into
microsoft:masterfrom
alan-agius4:feature/format-chains

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 commented Jan 5, 2018

Support formatting on chaining methods

Fixes #20996

@mhegazy mhegazy requested a review from a user January 10, 2018 18:17
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Jan 10, 2018

@Andy-MS mind reviewing this change

@ghost
Copy link
Copy Markdown

ghost commented Jan 10, 2018

I tested out the test file and it already passes in master. The test doesn't have any OpenBracketTokens so it makes sense that it's not affected by the change.

@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Jan 10, 2018

I just got typescript@next still the formatting is not correct unless I add this piece of code.

Checked a bit and it seems that it should be rightly so OpenParenToken as it's 19 but I am quite confused how the test is passing none-the-less.

Maybe I did something wrong with the test case? Cause I can see that my code is fixing the bug. I did a GIF demonstrating the issue and fix maybe I am missing something ;/

jan-10-2018 21-03-55p

@alan-agius4 alan-agius4 force-pushed the feature/format-chains branch from 82974ba to 14255cd Compare January 10, 2018 20:43
@ghost
Copy link
Copy Markdown

ghost commented Jan 10, 2018

You might want to test with format.document(); verify.currentFileContentIs(...); in addition to verify.indentationAtPositionIs tests.

@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Jan 10, 2018

Okay thanks for the pointers, I'll have a look tomorrow as also with my change now I broke 2 test haha. :), and sorry for the trouble.

@alan-agius4 alan-agius4 force-pushed the feature/format-chains branch 7 times, most recently from 782e43d to 98939a7 Compare January 12, 2018 11:22
@alan-agius4
Copy link
Copy Markdown
Contributor Author

Hey @Andy-MS I think I finally figured it out. I pushed some changes. Can you please review it again?

Thanks!

@alan-agius4 alan-agius4 force-pushed the feature/format-chains branch 2 times, most recently from 62cdc51 to 20b5381 Compare January 12, 2018 13:34
@alan-agius4 alan-agius4 force-pushed the feature/format-chains branch from 20b5381 to 0e0d3e4 Compare January 12, 2018 16:30
@alan-agius4
Copy link
Copy Markdown
Contributor Author

@Andy-MS reminder.

@ghost ghost merged commit bcb9fd7 into microsoft:master Jan 16, 2018
@alan-agius4 alan-agius4 deleted the feature/format-chains branch January 16, 2018 18:58
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
This pull request was closed.
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.

2 participants