Bug 340166

Comment 1 Moritz Eysholdt 2011-03-29 08:59:15 EDT

ok, I'll take a look at this later this week.

Comment 2 Henrik Lindberg 2011-03-29 11:07:31 EDT

(In reply to comment #1)
> ok, I'll take a look at this later this week.

Fantastic, we are trying to wrap up a release and do screen cams. Being able to include showing formatting as well would be awesome.

Please let me know if I can help with anything (and ping me on IRC or Skype if needed).

Comment 3 Moritz Eysholdt 2011-04-01 13:09:47 EDT

Hi Hendrik,

thanks a lot for the extensive test cases. They unveiled bug 341647, which I have fixed today. This turns 3 of your test cases green. I'll look at the others next week.

regards,
  Moritz

Comment 4 Moritz Eysholdt 2011-04-04 10:59:30 EDT

Next unveiled and fixed bug in line:
bug 341797 - [formatter] ElementMatcher: first parser rule call is not put on stack.

7 tests are green now, 3 to go.

Comment 5 Moritz Eysholdt 2011-04-04 11:13:57 EDT

TestFailingSerialization.test_Serialize_DqStringFollowedByDefine()
failes because of:

---- Expected --
import "foo"
define b {
	$a = 1
}
----------------

---- Actual ----
import "foo"
define b {
	$a = 1 }
----------------


you can fix this by removing the following statement form the FomattingConfig:
---
c.setNoLinewrap().between(ga.getLiteralNameOrReferenceRule(), ga.getExpressionListRule());
---

The cause is that setNoLinewrap() has precedence over setLinewrap()

Comment 6 Moritz Eysholdt 2011-04-04 11:36:09 EDT

For TestFailingSerialization.test_Serialize_ManifestStatements(), the test fails with:

---- Expected -----
include 'a'
a
-------------------


---- Actual -------
include
'a'
a
-------------------

This is because the parser actually parses three "ExpressionList"s. And therefore the formatting rule 
---
c.setLinewrap().after(ga.getPuppetManifestAccess().getStatementsAssignment_1());
---
kicks in.

Since this issue is not formatting related, I'll leave it to you to fix this.

It may be helpful to use NodeModelUtils in the TestCases
---
XtextResource r = getResourceFromString(code);		System.out.println(NodeModelUtils.compactDump(r.getParseResult().getRootNode(), false));
---
to find out what the parser actually reads.

Comment 7 Moritz Eysholdt 2011-04-04 12:08:25 EDT

The failure of 
TestFailingSerialization.test_Serialize_IfExpression1()
is cause by:
bug 341815 - "[serializer] NodeModel's nodes sometimes not correctly associated with ParseTreeConstructors tokens"

Getting the tokens and nodes to play along is not really an issue I'd like to dive into, especially since 
bug 333976 - "[Serializer] replace backtracking algorithm with declarative approach"
promises to fix that issue once and for all.

Comment 8 Moritz Eysholdt 2011-04-04 12:11:22 EDT

closing this bug since all issues directly related to formatting (except preserving formatting, but that will be handled by bug 341815) have been addressed.

Henrik, I think you owe me a beer ;)

Comment 9 Henrik Lindberg 2011-04-04 16:15:25 EDT

(In reply to comment #8)
> closing this bug since all issues directly related to formatting (except
> preserving formatting, but that will be handled by bug 341815) have been
> addressed.
> 
Thanks for the fixes, and the detective work. The compactDump was a good tip!

> Henrik, I think you owe me a beer ;)
Between you and Sebastian, I think I owe a case by now :)

Cheers.