Formatting of launch configuration files not possible

[wnickl]

Using CTRL+SHIFT+F in eclipse results in "complete content of file is moved to first line of file".

So instead of

java configuration XXX : BaseJavaLog4jLaunch {
memory max = 1280M;
main-class com.test.something.Main;
project com.test.something;
argument "42";
vm-argument "-Dtarget.home=${target_home}" ;
}

I get

java configuration XXX : BaseJavaLog4jLaunch { memory max = 1280M; main-class om.test.something.Main; project com.test.something; argument "42"; vm-argument -Dtarget.home=${target_home}" ; }

[miklossy]

Such a result usually comes if the Xtext default formatter is used, but in LcDsl, a custom formatter implementation is already bound in the AbstractLcDslRuntimeModule, so I think this issue has been solved in the meantime and can be closed.

[mduft]

Yes, a formatter is bound and in place. However it does not fully work, and I haven't had time to look into the issue. It seems that indentation is only correctly applied for the first line of a block, but not for the others...

[miklossy]

Thanks for the info. You are right, I experienced the same behaviour. I will take a look at it and try to provide a pull request to improve the formatter.

[mduft]

This would be really appreciated, thanks!

[miklossy]

Hi Markus,

I analysed the problem and found out that in order to make the indentation properly work, the formatter should append a newline to each line. A quick solution could be to override the void format(EObject it, extension IFormattableDocument document) method in the LcDslFormatter class:

class LcDslFormatter extends AbstractFormatter2 {
    ...
    override dispatch void format(EObject it, extension IFormattableDocument document) {
        regionForEObject.allSemanticRegions.last.append[newLine]
    }
}

However, it could result in some unwanted side-effects when applying this customization to all EObjects. Do you have the possibility to test it on some *.lc files?

Thanks a lot for you feedback!
Tamás

[mduft]

Hey,

Thanks for the analysis! I can certainly test this, yes :) It will just take me a little time to do so...

Cheers,
Markus

[mduft]

I tried this out quickly, and it seems that once I remove indentation on any line within a launch configuration, format will not bring it back :| It seems this kind of override is also never called, since the other dispatch methods in the file get precedence when they match - at least a Syserr in the method was never called.

[miklossy]

Hmm.. interesting. Actually it seems to work on my computer:

[miklossy]

@mduft Could it be possible that your code base is different from that what is checked in in the master branch? https://github.com/mduft/lcdsl/blob/master/com.wamas.ide.launching/src/com/wamas/ide/launching/formatting2/LcDslFormatter.xtend

[mduft]

Nope, I just freshly cloned, maybe something else went wrong... You just added that method as it is shown at the end of the file, right? Because that's what I tried :D

[mduft]

Ah, I think I found it - will re-test :) There was a problem with my setup.

[mduft]

OK, now it works. The only problem I observed when formatting our lc files is that blank lines disappear... We have a lot of blank lines separating blocks of arguments belonging together, blank lines before comments, etc.

[miklossy]

Preserving the already present new lines can be controlled by the setNewLines(int minNewLines, int defaultNewLines, int maxNewLines) parameters.

	/**
	 * Configures the given new lines for this hidden region. Keeps the current configuration if it is in the valid
	 * boundaries of {@code minNewLines} and {@code maxNewLines}. Applies {@code defaultNewLines} otherwise.
	 */
	void setNewLines(int minNewLines, int defaultNewLines, int maxNewLines);

Instead of calling regionForEObject.allSemanticRegions.last.append[newLine], you can call e.g.
regionForEObject.allSemanticRegions.last.append[setNewLines(1,1,2)]

Hint: In order to avoid org.eclipse.xtext.formatting2.ConflictingFormattingException: Conflicting values, you may have to adapt the [newline] parameters on other places as well.

[mduft]

Nice, thanks :) Can you provide a pull request? I would otherwise try to squeeze the changes in somewhere in the next days...

[miklossy]

Yes, I will provide a PR soon.

[mduft]

This is great news, thanks :)

[miklossy]

I created the PR

[mduft]

Thanks a lot. Will release ASAP.

[mduft]

New build is available at https://mduft.github.io/lcdsl-latest/ - you can verify (and hopefully enjoy) your changes there :)