|Formatting2 experiences [message #1749603]
||Thu, 08 December 2016 14:14
| Lieven Lemiengre
Registered: September 2010
We've just ported our existing formatter to the new 'formatting2' implementation. I think it's interesting to give the xtext developers some feedback |
Our formatter has a ton of features, so I think it pushes the framwork pretty hard, we have complicated vertical alignment, adaptive styles, various configurable options. Our grammar is big and complex and the files that we format can be large, the longest in our testcase is ~27k lines long.
First of all, we're pretty happy with the new implementation, our previous implementation required invasive changes in the framework, the new implementation only requires some shallow customization. We did have a few issues, not deal-breakers but annoyances nonetheless.
1) The formatter implementation for our grammar is rather big (~2400 LOC). Editing this file is not a pleasant experience. Auto-complete is very slow ~3-5 sec and there are many small pauses while we edit the file on my hi-end machine . The formatter 'dsl' uses a lot of extension methods & lambdas, this pushes the xtend compiler pretty hard. I'm not sure how this could be solved, maybe the performance of the xtend compiler can be improved? I don't think it's desirable to split the file.
2) To determine alignment we often have to figure out the size of a prefix, for example:
// a simple assignment statement
// before formatting
result ( 2*x to 10) <= 2*some_function ( arg1<=foo,
arg2 <= bar );
// after formatting
result(2 * x to 10) <= 2 * some_function(arg1 <= foo,
arg2 <= bar);
To determine the alignment for arg2 we need to find out the size of "result(2 * x to 10) <= 2 * some_function(", but we can only know this after we've formatted this statement. Currently we overcome this issue by cutting out the statement text, re-parsing it, formatting it with alignment disabled and then we figure out the size of the prefix. We didn't manage to figure out a method to call the formatter on a small part of the existing ast & node model without causing an IllegalArgumentException in NodeModelBasedRegionAccessBuilder.process, maybe it could be interesting to add this functionality to the framework? Calling the parser > 1000x causes some performance issues, Xtext parsers aren't optimized for very small files. When we format, about half the time is spend in AbstractInternalAntlrParser.registerRules. I managed to overcome this issue with some caching but I think it would be better if we could eliminate the re-parsing completely.
We're happy with the performance of the new formatter, after a day of optimization we managed to get the same performance as the old formatter. I did file 2 issues:
3.1) https://bugs.eclipse.org/bugs/show_bug.cgi?id=508892 aplying > 100k ITextReplacement freezes editor for minutes
3.2) https://bugs.eclipse.org/bugs/show_bug.cgi?id=508894 RegionTrace isn't cheap
4) Single line comments after code removes newlines when formatted: see https://github.com/eclipse/xtext-xtend/issues/77
Powered by FUDForum
. Page generated in 0.02361 seconds