eclipse-xtext / eclipse-xtext/xtext

Autowrap of formatter in Xtext 2.11 behaves wrongly in case of new lines

Open
#2,583 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Java
Stars
831
Forks
330
Avg merge
3d 7h
Merged PRs (30d)
12

Description

Assuming: maximal width is 120 characters

Input source code

class LooooooooooongC {

    looooooooooooooooongField:
LooooooooooongC |
             LooooooooooongC
&                        LoooooooooooongI |           LoooooooooooongI      &


LooooooooooongC


                        | LooooooooooongC                   & LoooooooooooongI | LooooooooooongC;
}

In our test case, the new formatter (Xtext 2.11) outputs the following wrong result

class LooooooooooongC {

	looooooooooooooooongField: LooooooooooongC | LooooooooooongC & LoooooooooooongI | LoooooooooooongI & LooooooooooongC | LooooooooooongC
		& LoooooooooooongI | LooooooooooongC;
}

Note that line has 139 characters and thus exceeds the maximal allowed width 120.

The older formatter of Xtext 2.10 behaves correctly in this case.

I believe, this has been introduced by the
commit

In particular, the following line could be problematic:

if (wrappable != null && context.isWrapInRegion()) {
				wrappable = null;
			} else {

The list of text replacements generated by the formatter looks the following

--- */
class<723| >LooooooooooongC<739| >{<741:6|

	>looooooooooooooooongField<772|>:<773:1| >LooooooooooongC<789| >|<791:14| >LooooooooooongC<820:1| >&<822:24| >LoooooooooooongI<862| >|<864:11| >LoooooooooooongI<891:6| >&<898:3| >LooooooooooongC<916:27| >|<944| >LooooooooooongC<960:19|
		>&<980| >LoooooooooooongI<997| >|<999| >LooooooooooongC<1015|>;<1016|
>}<1018|
>
--------------------------------------------------------------------------------
 723  1 " " -> " "
 739  1 " " -> " "
 741  6 "\n\n    " -> "\n\n	"
 772  0 "" -> ""
 773  1 "\n" -> " "
 789  1 " " -> " "
 791 14 "\n             " -> " "
 820  1 "\n" -> " "
 822 24 "                 ..." -> " "
 862  1 " " -> " "
 864 11 "           " -> " "
 891  6 "      " -> " "
 898  3 "\n\n\n" -> " "
 916 27 "\n\n\n              ..." -> " "
 944  1 " " -> " "
 960 19 "                   " -> "\n		"
 980  1 " " -> " "
 997  1 " " -> " "
 999  1 " " -> " "
1015  0 "" -> ""
1016  1 "\n" -> "\n"
1018  1 "\n" -> "\n"
---

The offset 891 is wrappable but the following offset 898 contains new lines and the formatter wrongly consider those new lines as wrapped and set wrappable to null even though these new lines are replaced by whitespaces later on.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with commit 7c7e1f88b744c3480f5ae10971efae467d50b9be and inspect the formatter logic around the wrappable and context.isWrapInRegion() condition. Reproduce the supplied input with a maximal width of 120 and compare the generated text replacements. Done means the formatted output does not exceed the configured width and the replacement list handles newlines that are later replaced by spaces.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.