Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: provide API to add blank lines #1268

Open
schakko opened this issue Apr 20, 2017 · 13 comments
Open

Feature request: provide API to add blank lines #1268

schakko opened this issue Apr 20, 2017 · 13 comments

Comments

@schakko
Copy link

schakko commented Apr 20, 2017

This is a simple feature request with no real business value but makes it a lot more easier to read through the source code.

Usage

block.addStatement(getFactory().createCodeSnippetStatement("int i = 1"));
block.addStatement(getFactory().createBlankLine());
// or block.addBlankLine()
block.addStatement(getFactory().createCodeSnippetStatement("for (; i < 0; i++) {}));

Current result

int i = 1;
for (; i < 0; i++) {}

Expected result

int i = 1;

for (; i < 0; i++) {}
@surli
Copy link
Collaborator

surli commented Apr 20, 2017

Actually I don't agree with that kind of API:
block.addStatement(getFactory().createBlankLine());

because it would mean that the blank line will be contained in the model, whereas it's only a pretty-print consideration.
However, maybe you could have a look on the DefaultJavaPrettyPrinter to improve or customize it to render the code for your specific needs.

@surli surli added the feature label Apr 20, 2017
@monperrus
Copy link
Collaborator

block.addStatement(getFactory().createBlankLine());

This simple API looks good to me. Would you propose a tentative implementation in a pull request?

@pvojtechovsky
Copy link
Collaborator

I suggest to have formatting information OPTIONALLY configurable on level of each CtElement.

If the formatting information is not null then DJPP would use it instead of standard formatting.

The formatting information might consist of this information:

  • number of EOL before the element
  • number of extra TABs and SPACES before the element
  • number of EOL after the element

To have it memory efficient, we can share this complex structure between all elements, which has same formatting request.

@monperrus
Copy link
Collaborator

monperrus commented Oct 16, 2017 via email

@tdurieux
Copy link
Collaborator

For me the best way to handle it is to support standart Code Formatter Settings (xml file).
like this one: https://github.com/mantono/CodeFormatterSettings/blob/master/intellij/Java.xml

@pvojtechovsky
Copy link
Collaborator

@schakko What would you preffer?
A) to be able to add extra line break on selected model element
B) to be able to define formatting rule, which adds that extra line before each for cycle
C) ???

I guess it depends on the use case
UC1) to keep formatting of source unchanged to minimize differences in history of source file.
UC2) to format printed sources following customizable rules to pass the clients checkstyle rules.
UC3) to apply special formatting to specific place to higlight that code.

It would be nice to support all these usecases

@schakko
Copy link
Author

schakko commented Oct 16, 2017

Personally I'd prefer option A as I like to have control when to add a line break. For example, after initialization of local variables I always add a line break to visualize that business logic follows. A formatter setting can't handle this use case.

@ghost
Copy link

ghost commented Oct 20, 2017

Automated source code formatting cannot determine when to apply newlines for grouping blocks of code that are logically related. For example:

String[] tokens = key.split(XML_DELIM);
List<String> tokenList = new ArrayList(Arrays.asList(tokens));
tokenList.remove(tokenList.size() - 1);
LOG.fine( "..." );

String parentKey = buildString(tokenList);
LOG.fine( "..." );

Element parentElement = getElementMap().get(parentKey);
if (parentElement == null) {
    parentElement = createElementHierarchy(parentKey);
}
LOG.fine( "..." );

Usually I'd want a blank line before and after the if statement. In this case, I can understand why the author kept those lines together: those lines form a related, coherent snippet.

After processing with Spoon, the code becomes:

String[] tokens = key.split(XML_DELIM);
List<String> tokenList = new ArrayList(Arrays.asList(tokens));
tokenList.remove(tokenList.size() - 1);
LOG.fine( "..." );
String parentKey = buildString(tokenList);
LOG.fine( "..." );
Element parentElement = getElementMap().get(parentKey);
if (parentElement == null) {
    parentElement = createElementHierarchy(parentKey);
}
LOG.fine( "..." );

This "wall of text" loses information.

it would mean that the blank line will be contained in the model, whereas it's only a pretty-print consideration.

Depends on how one views the model as it pertains to the problem domain. For an interpreter or compiler, whitespace and comments are superfluous. For a source code manipulation tool, whitespace is reasonably important to track. Although an AST doesn't generally include whitespace, line terminators, and comments, such lexical preservation is not unheard of. See also:

Having a CtBlankLine in the AST would help preserve layout in those cases where blank lines convey meaning that cannot be determined by an automated source beautifier.

@monperrus
Copy link
Collaborator

monperrus commented Oct 20, 2017 via email

@KishkinJ10
Copy link

i am a beginner can i work on this can i write this issue for my coding period in GSoC spoon project proposal?

@monperrus
Copy link
Collaborator

Talking about this feature with @MartinWitt today.

This feature would definitely be useful. In addition, it may be a solution to workaround some limitations of the sniper printer.

cc/ @bbaudry @Deee92

@monperrus
Copy link
Collaborator

per our meeting, is what you're looking for @I-Al-Istannen?

@I-Al-Istannen
Copy link
Collaborator

Yes, it would be helpful sometimes to create some visual separation. I think @MartinWitt did some stuff with custom AST elements which just cause a whitespace to be printed. I haven't really thought about a nice API for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants