Skip to content
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

PolicyOps: use newlines.ignoreInSyntax #3613

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Conversation

kitbellew
Copy link
Collaborator

Specifically, for PenalizeAllNewlines and SingleLineBlock policies. Fixes #3608.

@@ -2531,6 +2531,36 @@ newlines.inInterpolation
- `avoid`: attemps to avoid breaks within the spliced code, regardless of line overflow
- `oneline`: formats the splice on a single line, or breaks after `${` if overflows

### `newlines.ignoreInSyntax`

This boolean parameter controls whether to ignore newlines in strings (or other
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this only be used for strings? It's a bit unclear what other possibly multi-line tokens could entail. Overall, would that be a setting for that one user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. the code doesn't really specify which tokens could have embedded newlines; other possible options are comment and xml.
  2. as to the target audience: i would definitely use it in our codebase, not a big fan of ignored embedded newlines myself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgodzik any additional thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am missing the exact meaning here. whether to ignore newlines in strings yet the newlines are outside the strings, so this only affects strings if they are inside some other srtucture?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... let me add a longer explanation :)

Specifically, for PenalizeAllNewlines and SingleLineBlock policies.
Comment on lines +2581 to +2591
The formatter frequently chooses between adding a newline and continuing the
same line but either prohibiting or heavily discouraging subsequent newlines
_between_ tokens, to fit the rest of the expression on the same line.

However, in many cases and, for historical reasons, intentionally, newlines
_within_ tokens have been frequently ignored, leading to "single-line" blocks
which actually span multiple lines.

This boolean parameter now allows controlling whether to ignore newlines found
in syntax of strings or other possibly multi-line tokens when newlines are
otherwise prohibited or undesirable (such as for single-line formatting).
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgodzik do you think this explanation makes it a bit clearer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leading to "single-line" blocks which actually span multiple lines. is a bit unclear to me, though I guess the example here would need to be the best explanation we can get

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for expanding the explanation!

Comment on lines +2581 to +2591
The formatter frequently chooses between adding a newline and continuing the
same line but either prohibiting or heavily discouraging subsequent newlines
_between_ tokens, to fit the rest of the expression on the same line.

However, in many cases and, for historical reasons, intentionally, newlines
_within_ tokens have been frequently ignored, leading to "single-line" blocks
which actually span multiple lines.

This boolean parameter now allows controlling whether to ignore newlines found
in syntax of strings or other possibly multi-line tokens when newlines are
otherwise prohibited or undesirable (such as for single-line formatting).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leading to "single-line" blocks which actually span multiple lines. is a bit unclear to me, though I guess the example here would need to be the best explanation we can get

newlines.ignoreInSyntax = true
---
// ignores newline in string, pretends everything fits on one line
println(s"""${1}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an example. contains newline but will be formatted without breaks around open and close parentheses.

@kitbellew kitbellew merged commit 0c0dbfe into scalameta:master Aug 23, 2023
8 checks passed
@kitbellew kitbellew deleted the 3608 branch August 23, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insert newline / align closing paren after method call with single multiline string
2 participants