Eclipse Community Forums: TMF (Xtext) » Serialization: Indent missing at start of semantic modification

Help

Home

Home » Modeling » TMF (Xtext) » Serialization: Indent missing at start of semantic modification

Show: Today's Messages :: Show Polls :: Message Navigator

Serialization: Indent missing at start of semantic modification [message #1746528]

Mon, 31 October 2016 17:17

Stephan Herrmann

Messages: 1853
Registered: July 2009

Senior Member

This looks like room for improvement in the serializer, but let me check with you before filing a bug:

I have a model of this structure

module E {
    entity a {}
}

I have a quick fix that should add more entities, which is implemented as a semantic modification. The result of applying the quick fix is:

module E {
    entity a {}
entity b {}
    entity c {}
}

Problem: entity b is not properly indented. Formatter seems to be OK as Ctrl-Shift-F corrects the problem.

I could narrow it down to problematic interaction between HiddenTokenSequencer and FormattingConfigBasedStream:
When processing the '}' right before the insertion point for the new entities, we create a "preservedWS" containing just "\n".
Later during FormattingConfigBasedStream.Line.getSpacesStr(LineEntry, boolean), this preservedWS prevents computation of correct WS including indentation.

I could experimentally fix this inside HiddenTokenSequencer.getHiddenNodesBetween(): at the end of the (to == null) branch, I simply remove any trailing whitespace from out (still ensuring that trailing comments are correctly preserved incl. previous whitespace).

			for (int i = out.size() - 1; i >= 0; i--) {
				if (!tokenUtil.isWhitespaceNode(out.get(i)))
					break;
				out.remove(i);
			}

This seems to ensure that at the transition from old to new elements (the latter not having any nodes, yet) we don't spill preservedWS into the next (yet unformatted) element.

Does this make sense wrt the design of serialization? If so, are bugs in bugzilla still accepted (I don't have a github account)?

best,
Stephan

Report message to a moderator

Re: Serialization: Indent missing at start of semantic modification [message #1746562 is a reply to message #1746528]

Mon, 31 October 2016 21:58

Christian Dietrich

Messages: 14665
Registered: July 2009

Senior Member

i can not reproduce this with a simple unit test (with 2.11).

Module:
	"module" name=ID "{"
		entities+=Entity*	
	"}";
	
Entity:
	'entity' name=ID '{''}';

import com.google.inject.Inject
import org.eclipse.xtext.serializer.ISerializer
import org.eclipse.xtext.testing.InjectWith
import org.eclipse.xtext.testing.XtextRunner
import org.eclipse.xtext.testing.util.ParseHelper
import org.junit.Test
import org.junit.runner.RunWith
import org.xtext.example.mydsl1.myDsl.Module
import org.xtext.example.mydsl1.myDsl.MyDslFactory

@RunWith(XtextRunner)
@InjectWith(MyDslInjectorProvider)
class DummyTest {
	
	@Inject
	extension ParseHelper<Module> 
	
	@Inject 
	extension ISerializer
	
	@Test
	def void testIt() {
		val m =
		'''
		module E {
		    entity a {}
		}
		'''.parse
		m.entities += MyDslFactory.eINSTANCE.createEntity=>[name="b"] 
		m.entities += MyDslFactory.eINSTANCE.createEntity=>[name="c"] 
		
		println(m.serialize(SaveOptions.newBuilder.format.options))
	}
	
}

class MyDslFormatter extends AbstractFormatter2 {
	
	@Inject extension MyDslGrammarAccess

	def dispatch void format(Module module, extension IFormattableDocument document) {
		module.regionFor.keyword("{").append[newLine]
		module.regionFor.keyword("}").prepend[newLine]
		interior(module.regionFor.keyword("{"),module.regionFor.keyword("}"))[indent]
		for (Entity entity : module.getEntities()) {
			entity.format;
		}
	}
	
	def dispatch void format(Entity e, extension IFormattableDocument document) {
		e.regionFor.keyword("entity").prepend[newLine]
	}
}

but i can reproduce it from a quickfix

can you please file a ticket? (bugzilla is ok although github issue is prefered)

Twitter : @chrdietrich
Blog : https://www.dietrich-it.de

[Updated on: Mon, 31 October 2016 22:20]

Report message to a moderator

Re: Serialization: Indent missing at start of semantic modification [message #1774683 is a reply to message #1746528]

Wed, 18 October 2017 15:19

Sven Karol

Messages: 2
Registered: October 2017

Junior Member

I think I face the same issue in 2.11 and 2.12: when appending new model elements to a list in a block-like structure in the way as it is described in the first post above via a quickfix, the first element is not indentated.

The described workaround helps but seems to be incomplete as I had to apply further changes to make it work. I override HiddenTokenSequencer.getHiddenNodesBetween(INode from, INode to) as follows:

protected List<INode> getHiddenNodesBetween(INode from, INode to) { 
        List<INode> result = super.getHiddenNodesBetween(from,to);
	
	// Avoid that the new node is placed into a single-line comment
	if( result != null && result.stream().anyMatch(node -> tokenUtil.isCommentNode(node))){
		return result;
	}
		
	if (result != null && to == null) {
		for (int i = result.size() - 1; i >= 0; i--) {
			if (!super.tokenUtil.isWhitespaceNode(result.get(i)))
				break;
			result.remove(i);
		}
	}
		
	// Note: just return 'null' or a fresh empty list seens to indent the previous existing element in the list for whatever reason.
	return result;
}

However, just removing the trailing whitespace tokens from the result list does not prevent the instantiation of a (potentially empty) StringHiddenRegion which is then marked as undefined=false. This is bad for a quickfix since, in the "quickfix mode" the formatter (formatter2 actually) seems to only format hidden regions with defined=true. To enforce that the region is marked in a quickfix-friendly way I changed emitHiddenTokens too by commenting out its last two lines:

protected void emitHiddenTokens(List<INode> hiddens) {
         /* code from the original implementation */
	//if (lastNonWhitespace)// FIXME: determine the whitespace rule correctly 
		//delegate.acceptWhitespace(hiddenTokenHelper.getWhitespaceRuleFor(null, ""), "", null);
}

This causes that acceptWhitespace is not invoked and the undefined field of the hidden region remains at value true.

As the underlying region model seems to enforce interleaving hidden and non-hidden regions (am I right?), simply adding an undefined hidden region after the defined one when creating the document edits for the quickfix seems not to be a viable option, also this is currently not supported by the semantic modification API (again, am I right?). Are there any plans to address this issue, or are such kinds of quickfixes better implemented using syntactic modifications (although I think that the formatter and semantic modifications are very handy components)?

Report message to a moderator