Forced newlines in formatter

Development
1

It seems that some AI’s love writing very dense code, and it makes things rather unreadable. It would be amazing if we could print in a way that adds white-spacing / makes the formatting consistent. Just like we remove double white-spacing in the formatter.

  module Details = {
    @react.component
    let make = (~input as {filePath}: input) => React.string(filePath)
  }
  module Body = {
    @react.component
    let make = (~input as {filePath, oldString, newString, replaceAll}: input) => <>
      <ToolView.Field label="file" value=filePath />
      <ToolView.Text
        label={replaceAll->Option.getOr(false) ? "replace all" : "replace"} text=oldString
      />
      {newString == ""
        ? <ToolView.Field label="with" value="(nothing)" />
        : <ToolView.Text label="with" text=newString />}
    </>
  }
  module Output = Tool.TextResult.Output
  let failed = Tool.TextResult.failed

I opened an issue / PR for this, but would love to hear from you all wether this is something you experience, and wether changing the formatter for this is worth it for you all.

  • No, keep as is!
  • Yes please!
  • Behind some flag (--strict or something similar)
  • Other (reply)

0 voters

2 Likes
2

I would have loved to have this even before AI but found it too invasive. It still is but maybe we have a stronger argument now in favor of it. And I hope we can do without a flag. There might be some valid use-cases where you want no new lines between multi-line expressions though, but even then, those could be ignored if we can identify them.

3

Yeah, agree, I’m of the opinion that the formatting should pretty much always be optimized for the git diff over some cases where a single line is subjectively more readable

1 Like
4

I fully agree with you, readable git diff is far more important even more now with AI.

I would just avoid flags / parameters and instead address the cases that are the most objectively too dense.

2 Likes
5

I don’t have this problem, at least with the models I’m using, but that’s a nice thing regardless. And regrading the flag, I’d prefer to have it for disabling it instead of enabling it.

1 Like
6

Update: We ended up merging this as-is, without the flag. After seeing the changes in our projects we all agreed this was a net-benefit and it was inserting the whitespace exactly where we’d expect it.

Given the many changes happening in Rescript 13 anyway, we figured we would ship it without the experimental flag and stick to the no-configuration way of doing things in the formatter. Thank you for all the feedback :raised_hands:

3 Likes