Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

[Printer] - Multiline string as first function argument breaks formatting #212

Closed
eWert-Online opened this issue Dec 24, 2020 · 5 comments · Fixed by #241
Closed

[Printer] - Multiline string as first function argument breaks formatting #212

eWert-Online opened this issue Dec 24, 2020 · 5 comments · Fixed by #241

Comments

@eWert-Online
Copy link

The following function call stays as it is, which is, what I expect.

showDialog(
  `Do you really want to leave this workspace?`,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)

As soon as I break the multiline string into multiple lines, for example like this:

showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)

It gets printed like this:

showDialog(`
  Do you really want to leave this workspace?
  `, ~danger=true, ~confirmText="Yes, I am sure!", ~onConfirm=() => ())

I would expect it to stay like the second example.
(When I move the multiline string to the last position, it gets printed as I would expect.)

Playground example

@jfrolich
Copy link

Yes, this is occurring in our codebase a lot and the current form without indentation is quite ugly. See #35.

@IwanKaramazow
Copy link
Contributor

@eWert-Online Thanks for the report, this is indeed a bug. Will look into this.

@jfrolich If you see other weird patterns in your codebase, feel free to report them. Will probably have a look at "indented strings" end of Q1 2021.

@jfrolich
Copy link

@IwanKaramazow: Great! This was also an issue with Reason, so not really a regression, but I feel like it's something that exists in almost any modern programming language so it would be good to have it for ergonomics.

IwanKaramazow pushed a commit that referenced this issue Jan 19, 2021
Fix #212

Sometimes one of the non-callback arguments will break in a function application where the first or last argument is a callback. There might be a single line comment in there, or a multiline string. We want to break all the arguments in this case for readability.

**before**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `, danger=true, ~confirmText="Yes, I am sure!", ~onConfirm={() => ()},
)
```

**after**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)
```
IwanKaramazow pushed a commit that referenced this issue Jan 19, 2021
Fix #212

Sometimes one of the non-callback arguments will break in a function application where the first or last argument is a callback. There might be a single line comment in there, or a multiline string. We want to break all the arguments in this case for readability.

**before**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `, danger=true, ~confirmText="Yes, I am sure!", ~onConfirm={() => ()},
)
```

**after**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)
```
IwanKaramazow added a commit that referenced this issue Jan 19, 2021
…ks (#241)

Fix #212

Sometimes one of the non-callback arguments will break in a function application where the first or last argument is a callback. There might be a single line comment in there, or a multiline string. We want to break all the arguments in this case for readability.

**before**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `, danger=true, ~confirmText="Yes, I am sure!", ~onConfirm={() => ()},
)
```

**after**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)
```
@IwanKaramazow
Copy link
Contributor

@eWert-Online Sorry for the delay, but this is now fixed in master. Thank you so much for the report. This was a complex edge case.

@jfrolich I haven't forgotten about "indented multiline strings", will tackle this at at later time. Jsx4 and custom operators are getting my full attention.

@jfrolich
Copy link

@IwanKaramazow: You're doing an amazing job with the syntax @IwanKaramazow, love the progress! 🙌

kevinbarabash pushed a commit to kevinbarabash/rescript-compiler that referenced this issue Dec 24, 2021
…ks (rescript-lang#241)

Fix rescript-lang/syntax#212

Sometimes one of the non-callback arguments will break in a function application where the first or last argument is a callback. There might be a single line comment in there, or a multiline string. We want to break all the arguments in this case for readability.

**before**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `, danger=true, ~confirmText="Yes, I am sure!", ~onConfirm={() => ()},
)
```

**after**
```rescript
showDialog(
  `
  Do you really want to leave this workspace?
  Some more text with detailed explanations...
  `,
  ~danger=true,
  ~confirmText="Yes, I am sure!",
  ~onConfirm={() => ()},
)
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants