Skip to content

Format false branch of ternary operation#5905

Closed
saschanaz wants to merge 4 commits intomicrosoft:masterfrom
saschanaz:formatTernary
Closed

Format false branch of ternary operation#5905
saschanaz wants to merge 4 commits intomicrosoft:masterfrom
saschanaz:formatTernary

Conversation

@saschanaz
Copy link
Copy Markdown
Contributor

@saschanaz saschanaz commented Dec 3, 2015

This PR includes #4757 (#4757 is now merged 👍)

Fixes #3676.

Current:

var v =
    0 ? 1 :
        2 ? 3 :
            4;

Fixed:

var v =
    0 ? 1 :
    2 ? 3 :
    4;

@saschanaz saschanaz mentioned this pull request Dec 8, 2015
Merged
@saschanaz saschanaz changed the title Format false branch of ternary operation based on #4757 Format false branch of ternary operation Dec 9, 2015
@DanielRosenwasser
Copy link
Copy Markdown
Member

DanielRosenwasser commented Feb 26, 2016

Sorry this didn't get looked at @saschanaz. @vladima @mhegazy should this change have been behind a flag? My feeling is that some people might want cascading ternaries.

@basarat
Copy link
Copy Markdown
Contributor

basarat commented Mar 2, 2016

👍 from me. Just today:

image

As soon as a branch has a sub branch its better to move to anything other than a ternary (generally a new function containing a switch with cases having a return) so nesting is not a use case I have 🌹

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 5, 2016

sorry for the super late response. doing a quick poll, this seems a fairly subjective issue. some like their ternaries cascading, others like them on the same level. this calls for a new formatting option :)
so let's add a new option and condition the indentation on this option.

@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented May 6, 2016 •
edited
Loading

Okay, that's fine 👍

@saschanaz saschanaz closed this Dec 17, 2016
@saschanaz saschanaz reopened this Dec 17, 2016
@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented Dec 17, 2016 •
edited
Loading

What's wrong with the Linter? :/

@saschanaz saschanaz closed this Dec 17, 2016
@saschanaz saschanaz reopened this Dec 17, 2016
@saschanaz saschanaz closed this Dec 18, 2016
@saschanaz saschanaz reopened this Dec 18, 2016
@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented Dec 18, 2016

Opened #13006 for linter error.

@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented Jan 1, 2017

Just a note: the new option is now added :D

@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented May 26, 2017

Rebased.

mhegazy
mhegazy reviewed May 26, 2017
View reviewed changes
Comment thread src/services/types.ts Outdated
mhegazy
mhegazy approved these changes May 26, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

just the change to server\protocol.ts and we should be good to go. sorry for the looooooong delay.

@saschanaz
Copy link
Copy Markdown
Contributor Author

saschanaz commented May 26, 2017 •
edited
Loading

Renamed the option name from indentInsideTernaryOperator to indentConditionalExpressionFalseBranch, to match how existing TS codes call it.

PS: Why can't I add 😄 to the PR approval 🤔

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Aug 2, 2018

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.

@saschanaz saschanaz mentioned this pull request Nov 5, 2018
Closed
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mhegazy mhegazy mhegazy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@saschanaz @DanielRosenwasser @basarat @mhegazy @typescript-bot @msftclas @msftgits