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

FormatOps: in one-arg-per-line, handle lbrace case #1722

Merged
merged 4 commits into from
Feb 24, 2020

Conversation

kitbellew
Copy link
Collaborator

The scalafmt error described in #1599 turns out to be mainly due to the rule which formats multiple method arguments one per line. Router would attempt to format , { combination as a newline followed by the entire block on a single line, or a space without additional rules instead.

Two problems with this approach:

  • because of some aspect of BestFirstSearch algorithm that I don't yet fully understand, having these two possible splits led to the search reaching a dead end before it attempted the space split
    • the expectation was that having additional splits could only lead to a search state exploded error, not this)
    • removing the newline split (leaving the space split only) solved the problem
  • also, in cases when the space split was chosen, the result wasn't guaranteed to have one argument per line

Instead:

  • force a newline for regular or partial functions
  • for all other blocks, modify the space rule to force breaks after { and before }.

Fixes #1599.

scala-repos diffs: kitbellew/scala-repos@e2042d0?w=1

@kitbellew
Copy link
Collaborator Author

@olafurpg would appreciate some insight on the behaviour of BestFirstSearch algorithm. why would it stop searching when there are two splits for a given token, but not when one of them is removed? i had been under impression that more splits might potentially lead to explosion of search state, not to pruning of the search tree.

@kitbellew
Copy link
Collaborator Author

@olafurpg @tanishiking @poslegm PTAL

@kitbellew kitbellew merged commit 0270bb3 into scalameta:master Feb 24, 2020
@kitbellew kitbellew deleted the 1722 branch February 24, 2020 22:11
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Mar 4, 2020
kitbellew added a commit that referenced this pull request Mar 5, 2020
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.

Unable to format file due to bug in Scalafmt
2 participants