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

Format false branch of ternary operation #5905

Closed
wants to merge 4 commits into from

Conversation

saschanaz
Copy link
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 changed the title Format false branch of ternary operation based on #4757 Format false branch of ternary operation Dec 9, 2015
@DanielRosenwasser
Copy link
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
Copy link
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
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
Contributor Author

saschanaz commented May 6, 2016

Okay, that's fine 👍

@saschanaz
Copy link
Contributor Author

saschanaz commented Dec 17, 2016

What's wrong with the Linter? :/

@saschanaz
Copy link
Contributor Author

Opened #13006 for linter error.

@saschanaz
Copy link
Contributor Author

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

@saschanaz
Copy link
Contributor Author

Rebased.

Copy link
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
Contributor Author

saschanaz commented May 26, 2017

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
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants