Skip to content

Support same-column sequence indentation in YAML#7135

Merged
timtebeek merged 8 commits intomainfrom
Jenson3210/yaml-sequence-indent
Mar 30, 2026
Merged

Support same-column sequence indentation in YAML#7135
timtebeek merged 8 commits intomainfrom
Jenson3210/yaml-sequence-indent

Conversation

@Jenson3210
Copy link
Copy Markdown
Contributor

@Jenson3210 Jenson3210 commented Mar 25, 2026

Summary

  • Adds indentedSequences boolean to IndentsStyle to distinguish between indented style ( - item) and same-column style (- item at parent key column)

  • Extends Autodetect to detect which sequence indentation style a document uses by comparing sequence entry indent with parent mapping entry indent

  • Updates IndentsVisitor to respect the detected style, placing dashes at the correct column and computing lastIndent so sibling mapping entries align properly

  • Fixes IndentsVisitor does not support same-column sequence indentation style #7125

Test plan

  • Existing IndentsTest tests pass (no regressions for indented style)
  • New SameColumnSequenceIndent nested test class: basic indent correction, preservation of correct indent, nested mappings, multiple entries, mixed-type sequences
  • New AutodetectTest: detects indented style, detects same-column style, defaults to indented when no sequences present
  • Full rewrite-yaml test suite passes

Adds `indentedSequences` property to `IndentsStyle` to distinguish
between indented (`  - item`) and same-column (`- item`) sequence
styles. Autodetection compares sequence entry indent with parent
mapping entry indent to determine the style in use.

Fixes #7125
Main added handling for dash-on-own-line (multiline firstIndent) in
the lastIndent computation. Resolved by combining both: use dashColumn
(which accounts for same-column vs indented style) in the new
linebreak/non-linebreak branching from main.
@timtebeek
Copy link
Copy Markdown
Member

@Jenson3210
Copy link
Copy Markdown
Contributor Author

no, AFAICS @greg-at-moderne changed the indentation after the hyphen.
This PR changes the indentation + detection of the indentation before the hyphen.

In a yaml:

some-sequence:
-
  some-value
some-sequence:
<~~My detection + indentation fix~~>-<~~A new line~~>
<~~Greg PR indentation fix for the hyphen on previous line~~>some-value

Copy link
Copy Markdown
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Great to see this properly fixed, thanks!

If I'm not mistaken then any changes to style classes require a coordinated roll out, right?

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Mar 27, 2026
@Jenson3210
Copy link
Copy Markdown
Contributor Author

Jenson3210 commented Mar 27, 2026

Good to see you watching out. Will make some changes to be safer and only work when it really exists but not crash when not.

Wrap the new style method call with evaluate() try/catch to handle
NoSuchMethodError when parent runtimes haven't updated yet. Defaults
to true (indented sequences) which matches pre-existing behavior.
Comment on lines 26 to 30
public class IndentsStyle implements YamlStyle {
private int indentSize;
int indentSize;

boolean indentedSequences;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an implied former one arg constructor that we need to make explicit to ease the transition?

Copy link
Copy Markdown
Contributor Author

@Jenson3210 Jenson3210 Mar 30, 2026

Choose a reason for hiding this comment

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

I do not know / fully understand when these are used and when not, but just added them (+ a null supporting one) for safety here

Copy link
Copy Markdown
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Approved with one more question, just to avoid getting NoSuchMethodError at runtime.

Add deprecated single-arg constructor and @JsonCreator two-arg
constructor with @nullable Boolean indentedSequences for coordinated
rollout. Old LSTs without the field deserialize with null, which
defaults to true (indented style).
@timtebeek timtebeek merged commit cd4cd01 into main Mar 30, 2026
1 check passed
@timtebeek timtebeek deleted the Jenson3210/yaml-sequence-indent branch March 30, 2026 12:39
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

IndentsVisitor does not support same-column sequence indentation style

2 participants