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

Fix text block indentation when the text block has no prefix #1165

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dweiss
Copy link
Contributor

@dweiss dweiss commented Sep 24, 2024

The simplest example is this:

public class A {
  public void foo() {
  String a = """
lorem
ipsum
""";

  String b = """
 lorem
 ipsum
 """;
 }
}

Both text blocks should be aligned after the formatter pass but a isn't:

public class A {
  public void foo() {
    String a =
        """
lorem
ipsum
""";

    String b =
        """
        lorem
        ipsum
        """;
  }
}

I think the StringWrapper used to apply some more logic to actually "deindent" text block strings but that logic isn't needed and is in fact wrong - I added a comment in the patch.

Comment on lines -200 to -205
int deindent =
initialLines.get(1).stripTrailing().length() - lines.get(0).stripTrailing().length();

int startColumn = lineMap.getColumnNumber(startPosition);
String prefix =
(deindent == 0 || lines.stream().anyMatch(x -> x.length() + startColumn > columnLimit))
Copy link
Contributor Author

@dweiss dweiss Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to compute "deindent" at all here, I think, since no text block is allowed to appear at the top level of the syntax tree (?). Even if we wanted to allow it, the condition should be

String prefix = (startColumn == 0 || lines.stream().anyMatch(x -> x.length() + startColumn > columnLimit)) 
   ...
``.

Comment on lines -51 to -53
lorem
ipsum
""";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected output in this test was incorrect, it should have an indent.

@cushon
Copy link
Collaborator

cushon commented Sep 26, 2024

This is currently intentional, although there's some ongoing internal discussion about what the Google Java Style Guide should say about text blocks, and the 'outdenting' behaviour may change.

I think this is the same issue as #1087 (comment)

@dweiss
Copy link
Contributor Author

dweiss commented Sep 26, 2024

Yes, it seems to be the same underlying issue (code condition). I find it hard to understand why text blocks without an indent prefix would have to left without any formatting - this is inconsistent and strange to me. I also think it can lead to destructive formatting changes when used with intellij's plugin (which doesn't apply text block formatting). A subsequent run of the preformatter leaves the code in a different state compared to one run through the formatter. I'd have to come up with an example of this but I'm sure it's possible because I seem to indent those text blocks manually all the time...

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

Successfully merging this pull request may close these issues.

2 participants