reasonml / reason-cli

Globally installable Reason toolchain.
MIT License
291 stars 23 forks source link

refmt should move ternary operators onto the next line as the first non-whitespace char #86

Open johnhaley81 opened 6 years ago

johnhaley81 commented 6 years ago

Would anybody else feel like it would be better if ternary operators went as the first character on a new line instead of the last character on the current line?

Currently refmt does this:

someCondition ?
  doTheThing() :
  doTheOtherThing()

I think it would be more readable if it was formatted like this:

someCondition
  ? doTheThing()
  : doTheOtherThing()

When you have a big block of react components that are conditionally displayed on a boolean, your code gets auto formatted to use a ternary operator and it's really hard to see where the ? and : start/stop.

As a possible added benefit, I think this is how eslint and prettier formats ternary statements so people coming over from JS land might find this more familiar.

johnhaley81 commented 6 years ago

An example of a difficult to read block of React code:

  render: _self =>
    isTheThingConfiguredCorrectly(
      someCondition1,
      someCondition2,
      someCondition3,
    ) ?
      <Notifications.Notification type_=`success>
        <Notifications.Title>
          (ReasonReact.string("The thing is configured correctly"))
        </Notifications.Title>
        <Notifications.Paragraph>
          (ReasonReact.string("The app is good to go. Have fun!"))
        </Notifications.Paragraph>
      </Notifications.Notification> :
      <Notifications.Notification type_=`warning>
        <Notifications.Title>
          (ReasonReact.string("The thing requires some configuration"))
        </Notifications.Title>
        <Notifications.Paragraph>
          (
            ReasonReact.string(
              "Click the below button to finish the setup for your account.",
            )
          )
        </Notifications.Paragraph>
        <Buttons.ButtonGroup>
          <Buttons.Button primary=true onClick=(e => e |. onConfigureClick)>
            (ReasonReact.string("Configure your account"))
          </Buttons.Button>
        </Buttons.ButtonGroup>
      </Notifications.Notification>