Skip to content

Better ternary operator support #329

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

Closed
ksmithbaylor opened this issue Dec 10, 2015 · 8 comments
Closed

Better ternary operator support #329

ksmithbaylor opened this issue Dec 10, 2015 · 8 comments

Comments

@ksmithbaylor
Copy link

I use ternary expressions a lot, as part of a more functional "assign-once" style. But I always have to manually adjust my indentation and make sure to never accidentally auto-indent the parts of my code that use them. Here's the indentation I would like to use:

// single-line values
const foo = condition ?
  trueValue :
  falseValue;
nextLineOfCode();
// alternate style
const foo = condition
  ? trueValue
  : falseValue;
nextLineOfCode();
// multi-line values
const foo = condition ? (
  moreComplex(
    multiLineStatement
  )
) : (
  otherMultilineStatement(
    ohNo
  )
);

However, the current implementation indents this code like so:

// single-line values
const foo = condition ?
  trueValue :
    falseValue;
    nextLineOfCode();
// alternate style
const foo = condition
  ? trueValue
  : falseValue;
  nextLineOfCode();
// multi-line values
const foo = condition ? (
  moreComplex(
    multiLineStatement
  )
) : (
otherMultilineStatement(
  ohNo
)
);
@iryan2
Copy link

iryan2 commented Feb 22, 2016

This is something I regularly run in to as well, just thought I'd voice my support for this issue.

@jslatts
Copy link

jslatts commented Feb 23, 2016

This is something that I run into as well. I have been running locally with this patch: jslatts/vim-javascript@b4b3936

It gets close, but doesn't totally handle some of my use cases with JSX. I also don't know if it has any adverse side effects since I am not familiar with vim-script. Happy to make it a PR if the maintainers think it is ok.

@amadeus
Copy link
Collaborator

amadeus commented Apr 11, 2016

The first example case you have provided should be fixed in develop, the other ones are still issues.

Part of the challenge with this is that one size may not be able to fit all, to do it right there might have to be various modes that the indent file is put into, like comma first mode or whatever.

Unfortunately, none of the maintainers know much about the indent side of things, it was an inherited piece of work from previous maintainers that are no longer around, and thus is not updated often.

bounceme added a commit to bounceme/vim-javascript that referenced this issue Apr 16, 2016
also a possible fix for the last example in pangloss#329 and pangloss#315
bounceme added a commit to bounceme/vim-javascript that referenced this issue Apr 16, 2016
also a possible fix for the last example in pangloss#329 and pangloss#315
bounceme added a commit to bounceme/vim-javascript that referenced this issue Apr 16, 2016
also a possible fix for the last example in pangloss#329 and pangloss#315
@bounceme
Copy link
Collaborator

I'm for merging @jslatts solution as long as there aren't bad side effects

@jslatts
Copy link

jslatts commented Apr 18, 2016

FWIW, I have been using my patch since February without any side effects (that I have noticed). It doesn't help most of the time because I tend to use the ternary statements with JSX elements which still format poorly.

@bounceme
Copy link
Collaborator

Maybe we could use the same logic with every type of operator first?

@amadeus
Copy link
Collaborator

amadeus commented Apr 18, 2016

@jslatts can you make a PR with this change? I would recommend rebasing develop into it first though.

bounceme added a commit to bounceme/vim-javascript that referenced this issue Apr 18, 2016
also a possible fix for the last example in pangloss#329 and pangloss#315
jslatts pushed a commit to jslatts/vim-javascript that referenced this issue Apr 19, 2016
@amadeus
Copy link
Collaborator

amadeus commented Apr 20, 2016

I think we can mark this issue resolved. All the issues described seem to be rectified in the develop branch.

There is a small caveat however, in the middle example:

const foo = condition
  ? trueValue
  : falseValue;
nextLineOfCode();

a manual tab indent will be required after every carriage return when typing the ternary since we can never determine if a developer has decided to rely on ASI or not.

However, the issue pointed out here:

const foo = condition
  ? trueValue
  : falseValue;
  nextLineOfCode();

Where the nextLineOfCode is indented on a new line, is no longer an issue!

@amadeus amadeus closed this as completed Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants