From 61dff04ab16fa763396cc4ee822cdda25b2676e3 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 8 Apr 2026 16:08:31 +0200 Subject: [PATCH] fix(chunk): corrected blank line insertion to preserve tight packing in sibling declarations - Corrected blank line insertion logic to preserve tight packing for sibling declarations when editing packed containers. - Extracted visible_child_chunks() and sibling_gap_has_blank_line() helpers to unify child filtering and gap detection logic. - Updated VARIABLE_TRAITS to mark variables as packed and addressable_leaf for consistent spacing behavior. - Changed Key chunk kind to use PACKED_LEAF_TRAITS for proper tight packing alignment. - Added regression tests for TOML table and TypeScript variable packing after insertions. --- crates/pi-natives/src/chunk/edit.rs | 126 ++++++++++++++++++++++++++-- crates/pi-natives/src/chunk/kind.rs | 10 ++- 2 files changed, 126 insertions(+), 10 deletions(-) diff --git a/crates/pi-natives/src/chunk/edit.rs b/crates/pi-natives/src/chunk/edit.rs index 8b95deaec..59b2cf6ad 100644 --- a/crates/pi-natives/src/chunk/edit.rs +++ b/crates/pi-natives/src/chunk/edit.rs @@ -1101,24 +1101,63 @@ fn container_has_interior_content(state: &ChunkStateInner, anchor: &ChunkNode) - .any(|line| !line.trim().is_empty()) } +fn visible_child_chunks<'a>( + state: &'a ChunkStateInner, + anchor: &'a ChunkNode, +) -> Vec<&'a ChunkNode> { + anchor + .children + .iter() + .filter_map(|child_path| { + state + .tree + .chunks + .iter() + .find(|chunk| chunk.path == *child_path) + }) + .filter(|child| child.kind != ChunkKind::Chunk) + .collect() +} + +fn sibling_gap_has_blank_line( + state: &ChunkStateInner, + left: &ChunkNode, + right: &ChunkNode, +) -> bool { + right.start_line > owned_container_end_line(state, left) + 1 +} + /// Returns true if a container's children should be separated by blank lines. /// Root-level children (functions, classes) and containers with non-leaf /// children (methods) want blank line spacing. Containers whose children are /// all packed declarations (struct fields, enum variants) are tightly packed. fn children_want_blank_line_spacing(state: &ChunkStateInner, anchor: &ChunkNode) -> bool { if anchor.path.is_empty() { + let root_children = visible_child_chunks(state, anchor); + if root_children.len() >= 2 { + return root_children + .windows(2) + .any(|pair| sibling_gap_has_blank_line(state, pair[0], pair[1])); + } return true; } if anchor.children.is_empty() { return true; } - let all_packed = anchor.children.iter().all(|child_path| { - state - .tree - .chunks - .iter() - .any(|c| c.path == *child_path && c.kind.traits().packed) - }); + + let visible_children = visible_child_chunks(state, anchor); + if visible_children.is_empty() { + return true; + } + if visible_children.len() >= 2 { + return visible_children + .windows(2) + .any(|pair| sibling_gap_has_blank_line(state, pair[0], pair[1])); + } + + let all_packed = visible_children + .iter() + .all(|child| child.kind.traits().packed); !all_packed } @@ -2249,6 +2288,79 @@ mod tests { ); } + #[test] + fn packed_toml_table_after_inserts_stay_tightly_packed() { + let source = "[dependencies]\nanyhow.workspace = true\nbytes.workspace = \ + true\nserde.workspace = true\nsolar-interface.workspace = \ + true\nparking_lot.workspace = true\nsolar-sema.workspace = \ + true\ntokio.workspace = true\ntracing.workspace = true\n"; + let state = state_for(source, "toml"); + + let result = apply_single_edit(&state, "Cargo.toml", EditOperation { + op: ChunkEditOp::After, + sel: Some("table_dependencies.key_parking_lot_workspace".to_owned()), + crc: None, + region: None, + content: Some("rayon.workspace = true\n".to_owned()), + find: None, + }); + + assert!( + result.diff_after.contains( + "parking_lot.workspace = true\nrayon.workspace = true\nsolar-sema.workspace = true" + ), + "{}", + result.diff_after + ); + assert!( + !result + .diff_after + .contains("parking_lot.workspace = true\n\nrayon.workspace = true"), + "{}", + result.diff_after + ); + assert!( + !result + .diff_after + .contains("rayon.workspace = true\n\nsolar-sema.workspace = true"), + "{}", + result.diff_after + ); + } + + #[test] + fn packed_top_level_typescript_variables_stay_tightly_packed() { + let source = "const a = 1;\nconst b = 2;\nconst c = 3;\n"; + let state = state_for(source, "typescript"); + + let result = apply_single_edit(&state, "test.ts", EditOperation { + op: ChunkEditOp::After, + sel: Some("var_b".to_owned()), + crc: None, + region: None, + content: Some("const bb = 22;\n".to_owned()), + find: None, + }); + + assert!( + result + .diff_after + .contains("const b = 2;\nconst bb = 22;\nconst c = 3;"), + "{}", + result.diff_after + ); + assert!( + !result.diff_after.contains("const b = 2;\n\nconst bb = 22;"), + "{}", + result.diff_after + ); + assert!( + !result.diff_after.contains("const bb = 22;\n\nconst c = 3;"), + "{}", + result.diff_after + ); + } + #[test] fn crc_mismatch_error_includes_fresh_chunk_context() { let source = "class Foo {\n bar() {\n return 1;\n }\n}\n"; diff --git a/crates/pi-natives/src/chunk/kind.rs b/crates/pi-natives/src/chunk/kind.rs index d501bad6c..54a7ac7a4 100644 --- a/crates/pi-natives/src/chunk/kind.rs +++ b/crates/pi-natives/src/chunk/kind.rs @@ -196,8 +196,12 @@ const PACKED_LEAF_TRAITS: ChunkTraits = const FUNCTION_TRAITS: ChunkTraits = ChunkTraits { summary: SummaryStyle::Function, ..DEFAULT_TRAITS }; -const VARIABLE_TRAITS: ChunkTraits = - ChunkTraits { summary: SummaryStyle::Variable, ..DEFAULT_TRAITS }; +const VARIABLE_TRAITS: ChunkTraits = ChunkTraits { + packed: true, + addressable_leaf: true, + summary: SummaryStyle::Variable, + ..DEFAULT_TRAITS +}; const IMPORTS_TRAITS: ChunkTraits = ChunkTraits { groupable: true, summary: SummaryStyle::Imports, ..DEFAULT_TRAITS }; @@ -429,7 +433,7 @@ impl ChunkKind { Self::Interpolation => &GROUP_TRAITS, Self::Item => &DEFAULT_TRAITS, Self::Join => &DEFAULT_TRAITS, - Self::Key => &DEFAULT_TRAITS, + Self::Key => &PACKED_LEAF_TRAITS, Self::KeyScripts => &DEFAULT_TRAITS, Self::Label => &DEFAULT_TRAITS, Self::Let => &DEFAULT_TRAITS,