Skip to content

fix(javascript): ternary with --use-tabs#3277

Closed
ikatyang wants to merge 8 commits intoprettier:masterfrom
ikatyang:fix/javascript-ternary-use-tabs
Closed

fix(javascript): ternary with --use-tabs#3277
ikatyang wants to merge 8 commits intoprettier:masterfrom
ikatyang:fix/javascript-ternary-use-tabs

Conversation

@ikatyang
Copy link
Copy Markdown
Member

@ikatyang ikatyang commented Nov 15, 2017

Fixes #2771

image

EDIT: After second thought, spaces should only be applied to the trailing parentheses, marking as WIP.

ideal v2
ideal
old

@ikatyang ikatyang changed the title fix(javascript): ternary with --use-tabs [WIP] fix(javascript): ternary with --use-tabs Nov 15, 2017
@lydell
Copy link
Copy Markdown
Member

lydell commented Nov 15, 2017

We have to read through #1026 (the PR that added tabs support) again, because as far as I remember all of this was discussed back then.

When using spaces, Prettier indents with 2 spaces + 1 indent level. When using tabs, there are three ways to go:

  • Indent with one tab.
  • Indent with two tabs.
  • Indent with 2 spaces + 1 tab.

As far as I remember, all of those were discussed with pros and cons, but I don't have the time right now to dig into it.

@ikatyang ikatyang changed the title [WIP] fix(javascript): ternary with --use-tabs fix(javascript): ternary with --use-tabs Nov 20, 2017
@ikatyang
Copy link
Copy Markdown
Member Author

  • before
    • let tabWidth = 2
      • <tab><2 space><tab><2 space> -> <tab><tab><tab><tab>
      • <tab><4 space><tab><2 space> -> <tab><tab><tab><tab>
    • let tabWidth = 4
      • <tab><2 space><tab><2 space> -> <tab><tab><tab><tab>
      • <tab><4 space><tab><2 space> -> <tab><tab><tab><tab>
  • after
    • let tabWidth = 2
      • <tab><2 space><tab><2 space> -> <tab><tab><tab><2 space>
      • <tab><4 space><tab><2 space> -> <tab><tab><tab><tab><2 space>
    • let tabWidth = 4
      • <tab><2 space><tab><2 space> -> <tab><tab><2 space>
      • <tab><4 space><tab><2 space> -> <tab><tab><tab><2 space>

image
image
image


Since the change in the 3rd screenshot is same as the current { useTabs: false, tabWidth: 4 } result, I think it's not a regression... right?

Prettier 1.8.2
Playground link

--tab-width 4

Input:

aaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbb : ccccccccccccccc ? ddddddddddddddd : eeeeeeeeeeeeeee ? fffffffffffffff : gggggggggggggggg

Output:

aaaaaaaaaaaaaaa
    ? bbbbbbbbbbbbbbbbbb
    : ccccccccccccccc
      ? ddddddddddddddd
      : eeeeeeeeeeeeeee ? fffffffffffffff : gggggggggggggggg;

@ikatyang ikatyang added the status:wip Pull requests that are a work in progress and should not be merged label Jan 9, 2018
@ikatyang
Copy link
Copy Markdown
Member Author

Rewrite in #3745.

@ikatyang ikatyang closed this Jan 15, 2018
@ikatyang ikatyang deleted the fix/javascript-ternary-use-tabs branch January 15, 2018 03:42
@ikatyang ikatyang removed the status:wip Pull requests that are a work in progress and should not be merged label Jan 15, 2018
@lock lock Bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 18, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants