prettier: Ternary formatting is a bit unexpected

In my experience having ? and : on the new line improves the readability (as it’s easier to immediately see them while scanning code). This becomes more obvious when you have something more than a single word, e.g. an example with a require:

const { configureStore } = process.env.NODE_ENV === `production` ?
  require(`./configureProdStore`) : // eslint-disable-line import/no-commonjs
  require(`./configureDevStore`); // eslint-disable-line import/no-commonjs

// compared to

const { configureStore } = process.env.NODE_ENV === `production`
  ? require(`./configureProdStore`) // eslint-disable-line import/no-commonjs
  : require(`./configureDevStore`); // eslint-disable-line import/no-commonjs

(In the former it kinda looks like it’s requiring both until I read the code more closely, while in the latter it’s immediately obvious while scanning the code that it’s a ternary)

Or with jsx:

<button
  onClick={e => this.toggleAdvanced(e)}
  className="advancedOptions-button"
>
  {this.state.showAdvanced ?
    <FormattedMessage
      id="allocator-vacate-index.hide-advanced-options"
      defaultMessage="Hide advanced options"
    /> :
    <FormattedMessage
      id="allocator-vacate-index.show-advanced-options"
      defaultMessage="Show advanced options"
    />}
</button>

// compared to

<button
  onClick={e => this.toggleAdvanced(e)}
  className="advancedOptions-button"
>
  {this.state.showAdvanced
    ? <FormattedMessage
        id="allocator-vacate-index.hide-advanced-options"
        defaultMessage="Hide advanced options"
      />
    : <FormattedMessage
        id="allocator-vacate-index.show-advanced-options"
        defaultMessage="Show advanced options"
      />}
</button>

I feel the latter is more obvious when quickly scanning through the code (though, I’m biased as that’s the way I’ve written them for a long time, so might just be because of experience that I scan them quicker).

Here’s an example of a ternary in my code:

      const prop =
        field === 'DurationOfLast1_2Signal' ? 'durationLast12' :
        field === 'DurationOfLast1_4Signal' ? 'durationLast14' :
        field === 'LeakFault' ? 'leakFault' :
        null;

And here is how Prettier reformats it:

      const prop = field === 'DurationOfLast1_2Signal'
        ? 'durationLast12'
        : field === 'DurationOfLast1_4Signal'
            ? 'durationLast14'
            : field === 'LeakFault'
                ? 'leakFault'
                : null;

I know it’s subjective, but I think my code is more readable. In my real code there are 9 ternaries, so the Prettier indentation gets pretty extreme.

The default indentation is 2 spaces, so why does it switch to 4-space indentation after the first ternary?

@mvolkmann I know it’s maybe just an example but you would be way better off just doing something like:

const fieldMap = {
  DurationOfLast1_2Signal: 'durationLast12',
  DurationOfLast1_4Signal: 'durationLast14',
  LeakFault: 'leakFault',
};

const prop = fieldMap[field];

that use of ternary’s is super confusing

Sad to hear this. I know everyone has their hot button issues. Conciseness of code is very important to me. For me this is likely a deal breaker and means I will not be using Prettier.

R. Mark Volkmann Object Computing, Inc.

On Feb 23, 2017, at 11:03 PM, James Long notifications@github.com wrote:

I don’t think we’re going to add a new config option for this, and I think it is better to use different code for multiple ternaries. I actually find the prettier code slightly clearer because the other format is kind of abusing ternaries. If you’re really after performance:

let prop = null; switch(field) { case ‘DurationOfLast1_2Signal’: prop = ‘durationLast12’; break; case ‘DurationOfLast1_4Signal’: prop = ‘durationLast14’; break; case ‘LeakFault’: prop = ‘leakFault’; break; } In many cases it’s probably best to abstract this out into a function, then you could just do:

function lookupField(field) { switch(field) { case ‘DurationOfLast1_2Signal’: return ‘durationLast12’; case ‘DurationOfLast1_4Signal’: return ‘durationLast14’; case ‘LeakFault’: return ‘leakFault’; } } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

I don’t think we’re going to add a new config option for this, and I think it is better to use different code for multiple ternaries. I actually find the prettier code slightly clearer because the other format is kind of abusing ternaries. If you’re really after performance:

let prop = null;
switch(field) {
  case 'DurationOfLast1_2Signal':
    prop = 'durationLast12';
    break;
  case 'DurationOfLast1_4Signal':
    prop = 'durationLast14';
    break;
  case 'LeakFault':
    prop = 'leakFault';
    break;
}

In many cases it’s probably best to abstract this out into a function, then you could just do:

function lookupField(field) {
  switch(field) {
    case 'DurationOfLast1_2Signal':
      return 'durationLast12';
    case 'DurationOfLast1_4Signal':
      return 'durationLast14';
    case 'LeakFault':
      return 'leakFault';
  }
}

@mvolkmann I’m willing to support such an option in my fork, if you or someone else can submit a PR 😀

@jlongster I think you totally need a saved reply for these cases where you reject. It’s wasting your precious time.