Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[typescript, flow]: double indent for unions inside of tuples #6381

Open
wants to merge 4 commits into
base: master
from

Conversation

@squidfunk
Copy link

commented Aug 11, 2019

This PR fixes indentation of multi-line union types inside of TypeScript/Flow tuples.

Input:

type B = [
  | AAAAAAAAAAAAAAAAAAAAAA
  | BBBBBBBBBBBBBBBBBBBBBB
  | CCCCCCCCCCCCCCCCCCCCCC
  | DDDDDDDDDDDDDDDDDDDDDD
]

type C = [
  | [AAAAAAAAAAAAAAAAAAAAAA | BBBBBBBBBBBBBBBBBBBBBB | CCCCCCCCCCCCCCCCCCCCCC | DDDDDDDDDDDDDDDDDDDDDD]
  | [AAAAAAAAAAAAAAAAAAAAAA | BBBBBBBBBBBBBBBBBBBBBB | CCCCCCCCCCCCCCCCCCCCCC | DDDDDDDDDDDDDDDDDDDDDD]
]

Output (before fix):

type B = [

    | AAAAAAAAAAAAAAAAAAAAAA
    | BBBBBBBBBBBBBBBBBBBBBB
    | CCCCCCCCCCCCCCCCCCCCCC
    | DDDDDDDDDDDDDDDDDDDDDD
];

type C = [

    | [

          | AAAAAAAAAAAAAAAAAAAAAA
          | BBBBBBBBBBBBBBBBBBBBBB
          | CCCCCCCCCCCCCCCCCCCCCC
          | DDDDDDDDDDDDDDDDDDDDDD
    ]
    | [

          | AAAAAAAAAAAAAAAAAAAAAA
          | BBBBBBBBBBBBBBBBBBBBBB
          | CCCCCCCCCCCCCCCCCCCCCC
          | DDDDDDDDDDDDDDDDDDDDDD
    ]
];

Output (after fix):

type B = [
  | AAAAAAAAAAAAAAAAAAAAAA
  | BBBBBBBBBBBBBBBBBBBBBB
  | CCCCCCCCCCCCCCCCCCCCCC
  | DDDDDDDDDDDDDDDDDDDDDD
];

type C = [
  | [
      | AAAAAAAAAAAAAAAAAAAAAA
      | BBBBBBBBBBBBBBBBBBBBBB
      | CCCCCCCCCCCCCCCCCCCCCC
      | DDDDDDDDDDDDDDDDDDDDDD
    ]
  | [
      | AAAAAAAAAAAAAAAAAAAAAA
      | BBBBBBBBBBBBBBBBBBBBBB
      | CCCCCCCCCCCCCCCCCCCCCC
      | DDDDDDDDDDDDDDDDDDDDDD
    ]
];

Fixes #4794

Try the playground for this PR

@squidfunk squidfunk changed the title fix(TypeScript): double indent for unions inside of tuples [typescript, flow]: double indent for unions inside of tuples Aug 11, 2019

@lipis

lipis approved these changes Aug 11, 2019

@evilebottnawi
Copy link
Member

left a comment

oh, we need update changelog

@squidfunk

This comment has been minimized.

Copy link
Author

commented Aug 12, 2019

@evilebottnawi shall I add some notes to CHANGELOG.unreleased.md?

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@squidfunk yes, please use template as do this for other problems, thanks

@squidfunk

This comment has been minimized.

Copy link
Author

commented Aug 12, 2019

Done!

@bakkot

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2019

Hm, this ends up a little confusing when there's multiple types in the tuple:

type A = [
  string,
  | AAAAAAAAAAAAAAAAAAAAAA
  | BBBBBBBBBBBBBBBBBBBBBB
  | CCCCCCCCCCCCCCCCCCCCCC,
  | DDDDDDDDDDDDDDDDDDDDDD
  | EEEEEEEEEEEEEEEEEEEEEE
  | FFFFFFFFFFFFFFFFFFFFFF,
  number,
];

I assume that's why its formatted on master as

type A = [
  string,

    | AAAAAAAAAAAAAAAAAAAAAA
    | BBBBBBBBBBBBBBBBBBBBBB
    | CCCCCCCCCCCCCCCCCCCCCC,

    | DDDDDDDDDDDDDDDDDDDDDD
    | EEEEEEEEEEEEEEEEEEEEEE
    | FFFFFFFFFFFFFFFFFFFFFF,
  number,
];

which is at least unambiguous at a glance.

@bakkot

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2019

@evilebottnawi I messed up the first link; fixed. The code in the comment was still correct (with print-width 60, anyway.)

@squidfunk

This comment has been minimized.

Copy link
Author

commented Aug 12, 2019

@bakkot I see your concerns. Can we somehow detect if there're multiple types in the tuple? If there are multiple entries, we could add newlines before and after unions, without indenting them:

type A = [
  string,

  | AAAAAAAAAAAAAAAAAAAAAA
  | BBBBBBBBBBBBBBBBBBBBBB
  | CCCCCCCCCCCCCCCCCCCCCC,

  | DDDDDDDDDDDDDDDDDDDDDD
  | EEEEEEEEEEEEEEEEEEEEEE
  | FFFFFFFFFFFFFFFFFFFFFF,

  number,
];

However, I don't know whether that's easily possible with the current architecture.

@bakkot

This comment has been minimized.

Copy link
Collaborator

commented Aug 12, 2019

@squidfunk Yeah, you can probably do that in the tuple printing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.