Bug 347782

Comment 1 Sebastian Zarnekow 2011-05-31 09:51:27 EDT

The formatter should use the line delimiter that is configured for the XtextDocument. See IDocumentExtension4#getDefaultLineDelimiter

Comment 2 Moritz Eysholdt 2011-09-07 05:58:44 EDT

IDocumentExtension4#getDefaultLineDelimiter:
it tries to take the line delimiter of the first line of the document, and if that is not possible it takes the value of System.getProperty("line.separator") it the value is valid (e.g. one of "\r", "\n", "\r\n").

However, the formatter can't call getDefaultLineDelimiter() since formatting also happens when there is no document available (e.g. on serialization).

Furthermore, in Eclipse line delimiters can be configured per project. 
see org.eclipse.ui.internal.ide.LineDelimiterEditor.getStoredValue().


So I guess the correct way to find the line delimiter would try these steps in this order:
1. if there is a node model, take the line delimiter that terminates the first line.
2. otherwise, if a line delimiter has been configured for a project, take this.
3. otherwise, use System.getProperty("line.separator").

Comment 3 Sebastian Zarnekow 2011-09-07 13:14:44 EDT

see also org.eclipse.ui.editors.text.FileDocumentProvider.getLineDelimiterPreference(IFile)

Comment 4 Sebastian Zarnekow 2011-09-20 08:14:25 EDT

*** Bug 358228 has been marked as a duplicate of this bug. ***

Comment 5 Karsten Thoms 2011-09-20 11:23:39 EDT

As a workaround for those who use 2.0.1 release version add this to your Formatter:
=========================================================
 
      @Override
      public ITokenStream createFormatterStream(String indent, ITokenStream out,
                  boolean preserveWhitespaces) {
            return new FormattingConfigBasedStreamExt(out, indent, getConfig(),
                        createMatcher(), hiddenTokenHelper, preserveWhitespaces);
      }
 
      // Workaround for bug: Formatter uses \n for newline, not System line
      // see the bug number 358228
      // separator
      class FormattingConfigBasedStreamExt extends FormattingConfigBasedStream {
 
            public FormattingConfigBasedStreamExt(ITokenStream out, String indentation,
                        FormattingConfig cfg, IElementMatcher<ElementPattern> matcher,
                        IHiddenTokenHelper hiddenTokenHelper, boolean preserveSpaces) {
                  super(out, indentation, cfg, matcher, hiddenTokenHelper, preserveSpaces);
            }
 
            @Override
            protected Line createLine(List<LineEntry> initialEntries, int leftover) {
                  return new LineExt(initialEntries, leftover);
            }
 
            class LineExt extends Line {
                  public LineExt (List<LineEntry> initialEntries, int leftover) {
                        super(initialEntries, leftover);
                  }
                 
                  protected String wrap(int lines, String indent) {
                        StringBuffer result = new StringBuffer(lines + indent.length());
                        for (int i = 0; i < lines; i++)
                             result.append(System.getProperty("line.separator"));
                        // do not indent too deep as there would be no space left
                        // for semantic information
                        int indentLength = indent.length();
                        while ((cfg.getCharsPerLine() * 2 / 3) < indentLength) {
                             indentLength = indentLength - cfg.getCharsPerLine() / 2;
                        }
                        if (indentLength != indent.length())
                             indent = indent.substring(0, indentLength);
                        result.append(indent);
                        return result.toString();
                  }
            }
      }

Comment 6 Sebastian Zarnekow 2011-11-09 14:46:35 EST

Not 2.1

Comment 7 Jan Koehnlein 2012-01-06 11:53:19 EST

We should use the new IWhitespaceInformationProvider (see bug 367919)

Comment 8 Jan Koehnlein 2012-01-17 09:04:07 EST

That's gonna be trickier than I initially thought, as we don't even have a resource URI in most of the formatting API. Best solution seems to be to deprecate IFormatter in favor of an IFormatterExtension that additionally takes the line spearator to create the formatter stream. As this will wrap up into the old parse tree constructor serializer, I feel like leaving this issue until we redesign the formatter. Opinions?

Comment 9 Jan Koehnlein 2012-03-02 06:01:55 EST

Fixes and tests pushed to MASTER.

I am a bit lost with test 
  FormatterTest.testLinewrapDatatypeRuleRef1()
which actually fails before actually reformatting the model, but given the disabled assertions I don't seem to be the only one ;-). I set it to @Ignore. 

Moritz could you have a look at it?

Comment 10 Knut Wannheden 2012-03-05 02:48:43 EST

I now get an NPE when running the GenerateXbase.mwe2 workflow:

112596 ERROR CompositeGeneratorFragment - 
java.lang.NullPointerException
	at org.eclipse.emf.ecore.resource.impl.ExtensibleURIConverterImpl.normalize(ExtensibleURIConverterImpl.java:397)
	at org.eclipse.xtext.EcoreUtil2.getNormalizedURI(EcoreUtil2.java:593)
	at org.eclipse.xtext.EcoreUtil2.getNormalizedResourceURI(EcoreUtil2.java:575)
	at org.eclipse.xtext.formatting.impl.AbstractDeclarativeFormatter.createFormatterStream(AbstractDeclarativeFormatter.java:73)
	at org.eclipse.xtext.parsetree.reconstr.Serializer.serialize(Serializer.java:58)
	at org.eclipse.xtext.parsetree.reconstr.Serializer.serialize(Serializer.java:67)
	at org.eclipse.xtext.resource.XtextResource.doSave(XtextResource.java:314)
	at org.eclipse.emf.ecore.resource.impl.ResourceImpl.save(ResourceImpl.java:1423)
	at org.eclipse.xtext.generator.parser.antlr.DebugAntlrGeneratorFragment.prettyPrint(DebugAntlrGeneratorFragment.java:57)
	at org.eclipse.xtext.generator.parser.antlr.DebugAntlrGeneratorFragment.generate(DebugAntlrGeneratorFragment.java:44)
	at org.eclipse.xtext.generator.CompositeGeneratorFragment.generate(CompositeGeneratorFragment.java:81)
	at org.eclipse.xtext.generator.LanguageConfig.generate(LanguageConfig.java:108)
	at org.eclipse.xtext.generator.Generator.generate(Generator.java:352)
	at org.eclipse.xtext.generator.Generator.invokeInternal(Generator.java:126)
	at org.eclipse.emf.mwe.core.lib.AbstractWorkflowComponent.invoke(AbstractWorkflowComponent.java:126)
	at org.eclipse.emf.mwe.core.lib.Mwe2Bridge.invoke(Mwe2Bridge.java:34)
	at org.eclipse.emf.mwe.core.lib.AbstractWorkflowComponent.invoke(AbstractWorkflowComponent.java:201)
	at org.eclipse.emf.mwe2.runtime.workflow.AbstractCompositeWorkflowComponent.invoke(AbstractCompositeWorkflowComponent.java:35)
	at org.eclipse.emf.mwe2.runtime.workflow.Workflow.run(Workflow.java:19)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:97)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:73)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:64)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:55)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Launcher.run(Mwe2Launcher.java:74)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Launcher.main(Mwe2Launcher.java:35)

The problem is that the resource created by the DebugAntlrGeneratorFragment doesn't have any URI, which seems wrong. I think we should set an URI explicitly in DebugAntlrGeneratorFragment#prettyPrint():

	XtextResource resource = injector.getInstance(XtextResource.class);
	resource.setURI(URI.createFileURI(absoluteGrammarFileName));

Comment 11 Sebastian Zarnekow 2012-03-06 04:57:38 EST

(In reply to comment #10)
> I now get an NPE when running the GenerateXbase.mwe2 workflow:
> 

Pushed to master

Comment 12 Jan Koehnlein 2012-03-06 05:29:13 EST

Resolved?

Comment 13 Sebastian Zarnekow 2012-03-06 05:32:57 EST

(In reply to comment #12)
> Resolved?

I thought this one is still pending? FormatterTest.testLinewrapDatatypeRuleRef1()

Comment 14 Moritz Eysholdt 2012-03-06 08:58:47 EST

(In reply to comment #13)
> (In reply to comment #12)
> > Resolved?
> 
> I thought this one is still pending?
> FormatterTest.testLinewrapDatatypeRuleRef1()

I've fixed that.