Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Format false branch of ternary operation#5905

Closed
saschanaz wants to merge 4 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
saschanaz:formatTernarysaschanaz/TypeScript:formatTernaryCopy head branch name to clipboard
Closed

Format false branch of ternary operation#5905
saschanaz wants to merge 4 commits into
microsoft:mastermicrosoft/TypeScript:masterfrom
saschanaz:formatTernarysaschanaz/TypeScript:formatTernaryCopy head branch name to clipboard

Conversation

@saschanaz

@saschanaz saschanaz commented Dec 3, 2015

Copy link
Copy Markdown
Contributor

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

DanielRosenwasser commented Feb 26, 2016

Copy link
Copy Markdown
Member

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

basarat commented Mar 2, 2016

Copy link
Copy Markdown
Contributor

👍 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

mhegazy commented May 5, 2016

Copy link
Copy Markdown
Contributor

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

saschanaz commented May 6, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Okay, that's fine 👍

@saschanaz saschanaz closed this Dec 17, 2016
@saschanaz saschanaz reopened this Dec 17, 2016
@saschanaz

saschanaz commented Dec 17, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

saschanaz commented Dec 18, 2016

Copy link
Copy Markdown
Contributor Author

Opened #13006 for linter error.

@saschanaz

saschanaz commented Jan 1, 2017

Copy link
Copy Markdown
Contributor Author

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

@saschanaz

saschanaz commented May 26, 2017

Copy link
Copy Markdown
Contributor Author

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

@mhegazy mhegazy left a comment

Copy link
Copy Markdown
Contributor

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

saschanaz commented May 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

typescript-bot commented Aug 2, 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.

@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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.