Skip to content

Conversation

crogoz
Copy link
Contributor

@crogoz crogoz commented Sep 25, 2025

Before this PR

  • Text blocks were not formatted.
  • Intellij would not wrap long strings/would not format textBlocks. Different behaviour when running the formatter via the Intellij plugin vs. the gradle task.

After this PR

Fixes: #930
Fixes: #1331

  • Text blocks get formatted, by adding the initial """ on the previous line, the text block gets reindented.
  • Same behaviour between the Intellij plugin & gradle task formatters.

Example of a re-format: https://pl.ntr/2x2
==COMMIT_MSG==
Format text blocks + Intellij reflows strings
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Sep 25, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Format text blocks + Intellij reflows strings

Check the box to generate changelog(s)

  • Generate changelog entry


@Override
protected float computeWidth() {
if (token.getTok().getOriginalText().startsWith(StringWrapper.TEXT_BLOCK_DELIMITER)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before - when the textBlock+methods would exceed 120 characters (maxWidthLimit), the block would look like this:

    """
    hello %s this exceeds 120 characters nasddnsadalsdaslda
    """
        .formatted("world");

now:

    """
    hello %s this exceeds 120 characters nasddnsadalsdaslda
    """.formatted("world");

if (!StringWrapper.linesNeedWrapping(options.maxLineLength(), formattedText, characterRanges)) {
return writeFormatReplacements(replacements);
}
String wrappedFormattedText = StringWrapper.wrap(options.maxLineLength(), formattedText, formatter);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same behaviour now as with the gradle formatter task:

.

The alternative would have been to directly call the same method in the Intellij plugin, however I think this would be a bit more optimal - because when there is no need to call the wrapper, we can simply return just the replacement ranges. Worst case happens here where we have to send back the full document range to be reformatted.

Comment on lines 123 to +124
throws FormatterException {
JCTree.JCCompilationUnit unit = parse(input, /* allowStringFolding= */ false);
String separator = Newlines.guessLineSeparator(input);
return new Reflower(columnLimit, input).getReflowReplacements();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried to keep this in line with pjf logic - refactored to be similar to pjf - as it easier to compare diffs.

Comment on lines +212 to +215
String prefix = (lineStartPosition + startColumn + 4 > startPosition ? "" : " ".repeat(4))
+ " ".repeat(startColumn);

StringBuilder output = new StringBuilder(initialLines.get(0));
Copy link
Contributor Author

@crogoz crogoz Sep 25, 2025

Choose a reason for hiding this comment

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

diverges from gjf to ensure we indent text blocks

* column limit or contain text blocks. Used by the Intellij plugin to check if we need to run StringWrapper.wrap.
* Keep this method and {@code needWrapping} in line.
* */
public static boolean linesNeedWrapping(int columnLimit, String input, RangeSet<Integer> initialRangesToChange) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pjf specific - we need to figure out if we need to wrap the current input in the rangeSet initialRangesToChange

@changelog-app
Copy link

changelog-app bot commented Sep 25, 2025

Successfully generated changelog entry!

What happened?

Your changelog entries have been stored in the database as part of our migration to ChangelogV3.

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

""" + """
bar
""";
String u = stringVariableOne + """
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately string concatenation is not great with text blocks and I am not sure if I can change it.
after running the AST formatter I am getting (The AST cannot change the indentation of the text block node):

    String u = stringVariableOne + """
            ...
            """ + stringVariableTwo + """
                    ...
                    """;

and then after I try to re-indent the text blocks and given that I am trying to indent based on the previous line, I am getting the block below.

For these kind of text block operations, people should try to keep the textBlocks indented at the same line with the rest of the operands and then the block will be properly indented

@crogoz
Copy link
Contributor Author

crogoz commented Sep 25, 2025

Depends on palantir/gradle-baseline#3276. Adding do not merge until the previous PR is merged

@message
Copy link

message commented Oct 1, 2025

Waiting

int lineEnd = end;
while (Newlines.hasNewlineAt(input, lineEnd) == -1) {
lineEnd++;
private void indentTextBlocks(TreeRangeMap<Integer, String> replacements, List<Tree> textBlocks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In general the changes in this PR look much better! I ran it on gradle-guide here and most of the changes are an improvement imo.

The only weirdness (on this repo at least) is around multiple text block arguments to the same method, example:

refactorFromTo("""
    someCode
    """, """
        otherCode
        """);

I think the second text block is getting indented out of line with the first because the first ending """ has been indented?

You can take this to it's logical extreme:

void lotsOfStringParams(String a, String b, String c, String d, String e, String f, String g) {}

void test() {
    lotsOfStringParams("""
        aaa
        """, """
            bbb
            """, """
                ccc
                """, """
                    ddd
                    """, """
                        eee
                        """, """
                            fff
                            """, """
                                ggg
                                """);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this might be the same as a concatenation issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you force it to put each arg on a new line by making one arg very long, it kinda looks better but stops the """, """ pattern:

    void lotsOfStringParams(String a, String aa, String b, String c) {}

    void test() {
        lotsOfStringParams(
                """
                aaa
                """,
                "sdklfjslfkjsadlkgjsdklfjsadlkfjsaklfjaskdsfkljsaklfjsadflkjsdafkljasdfklsjfklsajflkasjfsfjksflfj",
                """
                bbb
                """,
                """
                ccc
                """);
    }

Copy link
Contributor Author

@crogoz crogoz Oct 3, 2025

Choose a reason for hiding this comment

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

re: last example with the one long arg
I believe this has to do with how parameters for a method are going to be structured if the maxLineColumn would exceed - same behaviour if we had long variable names/long annotated variables.

      void test(
              String aaaaaaaaaa,
              @NotNull @Variable Ssdklfjslfkjsdasdadasasdsadsadasdasadadasddadsasdsadadasdadadasdas variable,
              String bbbbbb) {
          lotsOfStringParams(
                  aaaaaaaaaa,
                  sdklfjslfkjsdasdadasasdsadsadasdasadadasddadsasdsadadasdadadasdasadadasdasfasfasfasfas,
                  bbbbbb);
      }

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

Successfully merging this pull request may close these issues.

Destructive formatting of multiline string Text block get formatted ugly
3 participants