GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — List position based formatting by saschanaz · Pull Request #13574 · microsoft/TypeScript

Conversation

@saschanaz

@saschanaz Kagami Sascha Rosylight (saschanaz) commented Jan 19, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #5830, fixes #5890, fixes #6252, fixes #6320, fixes #6451, fixes #10681, fixes #13688, and maybe more.

Previous behavior:

foo.then
  <
  void // list does not add indentation
  >();

foo
  .then(
  () => {}
  );

function foo(
  param1: number,
  param2: number
): {
    attribute: number // incorrectly double indented
  } {
}

New behavior:

foo.then
  <
    void // every list adds indentation
  >();

foo
  .then( // list starts on single-indented line
    () => {} // new formatter is list-position-sensitive, its children are now double indented
  );

function foo(
  param1: number,
  param2: number
): { // list starts on non-indented line
  attribute: number // correctly single indented
} {
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tinganho, jwbay, aluanhaddad, alitaheri, kraftwer1, localvoid, zpdDG4gta8XKpMCd, yume-chan, SYip, SlurpTheo, and piotrwitek reacted with thumbs up emoji alitaheri and prasanthkarukkuvel reacted with heart emoji
All reactions
  • 👍 11 reactions
  • ❤️ 2 reactions
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Jan 19, 2017
@saschanaz Kagami Sascha Rosylight (saschanaz) mentioned this pull request Jan 19, 2017
Fix child element indentation in multiline lists #3827
Closed
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Jan 20, 2017
View reviewed changes
Comment thread src/services/formatting/formatting.ts
if ((<TypeReferenceNode>node).typeArguments === list) {
return SyntaxKind.LessThanToken;
}
case SyntaxKind.TypeLiteral:

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Jan 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is this an intended fallthrough?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@saschanaz Kagami Sascha Rosylight (saschanaz) Jan 21, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No, fixed now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@jwbay jwbay mentioned this pull request Jan 26, 2017
Callback functions don't have the expected indentation #13688
Closed
@WuTheFWasThat Jeff Wu (WuTheFWasThat) mentioned this pull request Mar 11, 2017
multi-line argument indentation #14596
Closed
@saschanaz Kagami Sascha Rosylight (saschanaz) mentioned this pull request Apr 11, 2017
Fix formatting of parameter list of a multiple-line call expression #14955
Closed
@saschanaz Kagami Sascha Rosylight (saschanaz) changed the title List based formatting List position based formatting Apr 14, 2017
zhengbli
Zhengbo Li (zhengbli) reviewed Apr 17, 2017
View reviewed changes
Comment thread src/services/formatting/formatting.ts
*/
interface DynamicIndentation {
getIndentationForToken(tokenLine: number, tokenKind: SyntaxKind, container: Node): number;
getIndentationForToken(tokenLine: number, tokenKind: SyntaxKind, container: Node, suppressDelta: boolean): number;

@zhengbli Zhengbo Li (zhengbli) Apr 14, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you add a comment for the "suppressDelta" parameter, including examples of when would that be set to true? So it would be easier to read

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@saschanaz Kagami Sascha Rosylight (saschanaz) Apr 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I added now 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/formatting/formatting.ts
consumeTokenAndAdvanceScanner(tokenInfo, parent, parentDynamicIndentation, parent);

if (indentationOnLastIndentedLine !== Constants.Unknown) {
indentationOnListStartToken = indentationOnLastIndentedLine;

@zhengbli Zhengbo Li (zhengbli) Apr 14, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you add a comment here explaining the relationship between indentationOnLastIndentedLine and indentationOnListStartToken? It looks like they are related somehow

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@saschanaz Kagami Sascha Rosylight (saschanaz) Apr 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Also added 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented May 29, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Test fails on codeFixClassImplementInterfaceComputedPropertyNameWellKnownSymbols because of a strange NodeArray object with the form [ pos: undefined, end: undefined, hasTrailingComma: undefined ]. Need some bisecting to solve this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented May 29, 2017

Copy link
Copy Markdown
Contributor Author

BTW I think the fix for codeFixClassImplementInterfaceComputedPropertyNameWellKnownSymbols is out of scope of this PR, a simple workaround may be enough?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request May 29, 2017
@saschanaz
workaround for codefix test failure
1e4358f 
See microsoft#13574 (comment)
@saschanaz
Kagami Sascha Rosylight (saschanaz) force-pushed the listIndent branch from 1e4358f to 8971bc7 Compare May 29, 2017 17:16
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request May 29, 2017
@saschanaz
workaround for codefix test failure
96d11e6 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added 7 commits May 30, 2017 02:34
@saschanaz
use last line indentation on formatter
8d1cfbd 
... and add explicit delta suppressor for list end tokens
@saschanaz
remove deprecated function
e92711c
@saschanaz
add getListByPosition
bfb1bf5
@saschanaz
indent using list start position before first list item
0464ee1
@saschanaz
indent using list start position
1541096
@saschanaz
tests
92d6566
@saschanaz
workaround for codefix test failure
cd2acf5 
See microsoft#13574 (comment)
@saschanaz
Kagami Sascha Rosylight (saschanaz) force-pushed the listIndent branch from 8971bc7 to cd2acf5 Compare May 29, 2017 18:26
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented May 29, 2017

Copy link
Copy Markdown
Contributor Author

Rebased and squashed for easier review. Sorry for previous mess 😅
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@magnushiie

Magnus Hiie (magnushiie) commented Aug 13, 2017

Copy link
Copy Markdown
Contributor

What is the status of this PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented Aug 14, 2017

Copy link
Copy Markdown
Contributor Author

I think it's mature enough, time to merge conflicts anyway...
localvoid reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@msftgits Microsoft GitHub User (msftgits) removed the cla-already-signed label Sep 26, 2017
@typescript-bot

TypeScript Bot (typescript-bot) commented Jun 13, 2018

Copy link
Copy Markdown
Contributor

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@typescript-bot TypeScript Bot (typescript-bot) closed this Jun 13, 2018
@mhegazy

Mohamed Hegazy (mhegazy) commented Jul 19, 2018

Copy link
Copy Markdown
Contributor

Andy (Andrewkraft) (@Andy-MS) can you refresh this PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy Mohamed Hegazy (mhegazy) reopened this Jul 19, 2018
@mhegazy Mohamed Hegazy (mhegazy) assigned ghost Jul 19, 2018
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Sep 5, 2018
Revive PR #13574 #26906
Closed
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) closed this Sep 5, 2018
@saschanaz Kagami Sascha Rosylight (saschanaz) mentioned this pull request Nov 3, 2018
Hard to reproduce older build environment #28327
Closed
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented Nov 3, 2018

Copy link
Copy Markdown
Contributor Author

Note for myself: downgrade to TS 2.4.x, gulp-typescript 3.0.x, and mocha 3.5.x to build this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
d28d0d1 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
87ecfa1 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
4fe3bfa 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
d6e8ddd 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
c6744a4 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
578a129 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
d68a218 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
0ebfb3a 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
8b20b3c 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
fd45b84 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
18810d2 
See microsoft#13574 (comment)
Kagami Sascha Rosylight (saschanaz) added a commit to saschanaz/TypeScript that referenced this pull request Nov 3, 2018
@saschanaz
workaround for codefix test failure
1556743 
See microsoft#13574 (comment)
@saschanaz

Kagami Sascha Rosylight (saschanaz) commented Nov 4, 2018

Copy link
Copy Markdown
Contributor Author

Test fails on codeFixClassImplementInterfaceComputedPropertyNameWellKnownSymbols

Solved by #18284
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@saschanaz Kagami Sascha Rosylight (saschanaz) mentioned this pull request Nov 5, 2018
List position based formatting #28340
Merged
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) RyanCavanaugh left review comments
+1 more reviewer
@zhengbli Zhengbo Li (zhengbli) zhengbli left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Indentation issues on multiline call expressions unexpected chained "then" method indentation formatting problem with an object literal as an inlined result type of a function angle brackets formatting problem fluent api style method chain formatting problem
Formatting with method parameters Callback functions don't have the expected indentation

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@saschanaz @magnushiie @typescript-bot @mhegazy @zhengbli @RyanCavanaugh @msftclas @msftgits
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.