diff --git a/CHANGELOG.md b/CHANGELOG.md index b749633b..524032fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,12 @@ The workspace `Cargo.toml` `[workspace.package] version` is the single source fo unchanged to preserve opaque schema/package dependencies; ordinary convert, edit and identity round-trips retain their existing history behavior ([#449](https://github.com/STAIxBWLB/hwp-cli/issues/449)). +- Anchored `insert_para` and `move_para` now refuse to place a paragraph in front of a section's + definition paragraph, with the same cause and remedy as the addressed `insert_blocks` guard: + the native writers cannot move the setup, so HWPX would inject a second default `secPr` and + HWP5 would leave the section without secd/cold. Insert with `before=false` and move behind the + owner instead ([#451](https://github.com/STAIxBWLB/hwp-cli/issues/451)). + - Shared stage-sync and file-identity helper failures now name their phase and advise checking output-directory permissions and free space before rerunning. The helper error values use fixed Korean messages without staging/destination paths or underlying I/O error chains, diff --git a/crates/hwp-cli/src/commands/edit.rs b/crates/hwp-cli/src/commands/edit.rs index ceeb032a..9217eff5 100644 --- a/crates/hwp-cli/src/commands/edit.rs +++ b/crates/hwp-cli/src/commands/edit.rs @@ -1448,7 +1448,10 @@ pub fn execute(input: &Path, output: &Path, plan: &EditPlan) -> anyhow::Result").with_context(|| { format!("--insert-para-before 형식은 \"앵커=>텍스트\" 입니다: {spec:?}") })?; - if hwp_convert::insert_paragraph(&mut doc, anchor, text, true) { + let inserted = + hwp_convert::insert_paragraph_checked(&mut doc, anchor, text, true) + .map_err(|error| anyhow::anyhow!(error))?; + if inserted { eprintln!("문단 삽입(앞): {anchor:?} 앞에 {text:?}"); edits += 1; } else { @@ -1462,7 +1465,10 @@ pub fn execute(input: &Path, output: &Path, plan: &EditPlan) -> anyhow::Result").with_context(|| { format!("--insert-para 형식은 \"앵커=>텍스트\" 입니다: {spec:?}") })?; - if hwp_convert::insert_paragraph(&mut doc, anchor, text, false) { + let inserted = + hwp_convert::insert_paragraph_checked(&mut doc, anchor, text, false) + .map_err(|error| anyhow::anyhow!(error))?; + if inserted { eprintln!("문단 삽입(뒤): {anchor:?} 뒤에 {text:?}"); edits += 1; } else { @@ -3664,12 +3670,18 @@ fn apply_typed_operation( eprintln!("문단 삽입(주소): {current_path} before={before} text={text:?}"); offsets.record_insert(&new_path); *edits += 1; - } else if hwp_convert::insert_paragraph(doc, anchor, text, *before) { - eprintln!("문단 삽입: {anchor:?}, before={before}, text={text:?}"); - *edits += 1; } else { - eprintln!("{}", paragraph_miss_message(doc, anchor, "앵커")); - unapplied.push(format!("insert_para anchor={anchor:?}")); + match hwp_convert::insert_paragraph_checked(doc, anchor, text, *before) { + Ok(true) => { + eprintln!("문단 삽입: {anchor:?}, before={before}, text={text:?}"); + *edits += 1; + } + Ok(false) => { + eprintln!("{}", paragraph_miss_message(doc, anchor, "앵커")); + unapplied.push(format!("insert_para anchor={anchor:?}")); + } + Err(error) => return Err(anyhow::anyhow!(error)), + } } } TypedEditOperation::DeletePara { matching, address } => { diff --git a/crates/hwp-cli/tests/edit_ops.rs b/crates/hwp-cli/tests/edit_ops.rs index d5713f37..f220b6eb 100644 --- a/crates/hwp-cli/tests/edit_ops.rs +++ b/crates/hwp-cli/tests/edit_ops.rs @@ -81,8 +81,9 @@ fn table_has_text(table: &Table, text: &str) -> bool { }) } -const TRACER_MD: &str = - "generated edit ops tracer fixture\n\na=>b\n\n| 가 | 나 |\n|---|---|\n| 1 | 2 |\n"; +/// The trailing paragraph is the tracer's `insert_para` anchor: `before:true` on the first +/// paragraph would now be refused, because that paragraph owns the section definition (#451). +const TRACER_MD: &str = "generated edit ops tracer fixture\n\na=>b\n\n| 가 | 나 |\n|---|---|\n| 1 | 2 |\n\ntracer tail paragraph\n"; /// The tracer fixture through `edit --ops` is lossless end to end: the "=>", "=" /// and ":" payloads inside the replace strings are data and land verbatim ("a=>b" @@ -128,7 +129,7 @@ fn tracer_ops_end_to_end_lossless() { .collect(); let anchor = texts .iter() - .position(|text| text == "generated edit ops tracer fixture") + .position(|text| text == "tracer tail paragraph") .expect("the anchor paragraph must survive"); let inserted = texts .iter() @@ -139,6 +140,11 @@ fn tracer_ops_end_to_end_lossless() { anchor, "before:true must insert immediately before the anchor" ); + assert_eq!( + texts.first().map(String::as_str), + Some("generated edit ops tracer fixture"), + "the section owner must stay the first paragraph" + ); assert!( texts.contains(&"c=d:e".to_string()), "the replace payload with =>, = and : must land verbatim, got {texts:?}" diff --git a/crates/hwp-cli/tests/insert_blocks.rs b/crates/hwp-cli/tests/insert_blocks.rs index 4b7c1cb4..dd023b40 100644 --- a/crates/hwp-cli/tests/insert_blocks.rs +++ b/crates/hwp-cli/tests/insert_blocks.rs @@ -1267,6 +1267,185 @@ fn insert_para_before_section_definition_refuses_and_after_succeeds() { section_owner_case(false); } +/// A base built by `hwp new --from` so the paragraph owning `Control::SectionDef` also carries +/// text the anchored selector can find. The composed fixture above keeps its section setup on an +/// empty first paragraph, which no anchor search can name, so it cannot show this boundary. +fn text_owner_base(label: &str, extension: &str) -> (PathBuf, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "hwp-section-owner-text-{}-{label}-{extension}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let markdown = dir.join("base.md"); + std::fs::write(&markdown, format!("{PREFIX}\n\n{ANCHOR}\n\n{TAIL}\n")).unwrap(); + let base = dir.join(format!("base.{extension}")); + success( + hwp() + .arg("new") + .arg("--from") + .arg(&markdown) + .arg("-o") + .arg(&base) + .output() + .unwrap(), + "markdown fixture with a text-owning section paragraph", + ); + (dir, base) +} + +/// Runs `hwp edit --ops` for a base this file built itself, mirroring `Fixture::edit`. +fn edit_file(base: &Path, dir: &Path, stem: &str, ops: Value) -> (PathBuf, Output) { + let ops_path = dir.join(format!("{stem}-ops.json")); + std::fs::write(&ops_path, serde_json::to_vec(&ops).unwrap()).unwrap(); + let output = base.with_file_name(format!( + "{stem}.{}", + base.extension().unwrap().to_string_lossy() + )); + let run = hwp() + .arg("edit") + .arg(base) + .arg("-o") + .arg(&output) + .arg("--ops") + .arg(ops_path) + .output() + .unwrap(); + (output, run) +} + +/// The anchored `insert_para` form (an `anchor` selector instead of an `address`) must refuse a +/// `before` insertion on the paragraph owning `Control::SectionDef`, with the same cause and +/// remedy as the addressed guard, in both native formats. Only the anchor search differs from +/// `insert_para_before_section_definition_refuses_and_after_succeeds`, so a search that inserts +/// without consulting the matched paragraph's controls fails at the diagnostic assertion. +#[test] +fn anchored_insert_para_before_section_definition_refuses_in_both_formats() { + for extension in ["hwp", "hwpx"] { + let (dir, base) = text_owner_base("anchor", extension); + let before = read(&base); + assert!( + before.sections[0].paragraphs[0] + .controls + .iter() + .any(|control| matches!(control, Control::SectionDef(_))), + "fixture must exercise the real section definition owner" + ); + assert_eq!( + index(&before, PREFIX), + 0, + "the anchor must name the section definition owner itself" + ); + let base_bytes = std::fs::read(&base).unwrap(); + + let (refused, run) = edit_file( + &base, + &dir, + "anchor-section-before", + json!([{"op":"insert_para","anchor":PREFIX,"before":true,"text":"Before section owner."}]), + ); + assert!( + !run.status.success(), + "insertion before the section definition owner must refuse" + ); + let diagnostic = String::from_utf8_lossy(&run.stderr).into_owned(); + assert!( + diagnostic.contains("구역") + && diagnostic.contains("앞") + && diagnostic.contains("before=false"), + "anchored refusal must carry the addressed guard's cause and remedy: {diagnostic}" + ); + assert!( + std::fs::read(&base).unwrap() == base_bytes, + "the refusal must not change the source document" + ); + assert!( + !refused.exists(), + "the refusal must not publish an output document" + ); + + let (output, run) = edit_file( + &base, + &dir, + "anchor-section-after", + json!([{"op":"insert_para","anchor":PREFIX,"before":false,"text":"After section owner."}]), + ); + success(run, "anchored insertion after the section definition owner"); + let after = read(&output); + assert_eq!(index(&after, "After section owner."), 1); + assert!( + after.sections[0].paragraphs[0] + .controls + .iter() + .any(|control| matches!(control, Control::SectionDef(_))) + ); + } +} + +/// `move_para` refuses a destination that would put another paragraph in front of the section +/// owner. Moving the owner itself already refused; this is the second path, so a destination +/// index of 0 on a section body must refuse before the source paragraph is removed. +#[test] +fn move_para_before_section_definition_refuses_in_both_formats() { + for extension in ["hwp", "hwpx"] { + let (dir, base) = text_owner_base("move", extension); + let before = read(&base); + assert!( + before.sections[0].paragraphs[0] + .controls + .iter() + .any(|control| matches!(control, Control::SectionDef(_))), + "fixture must exercise the real section definition owner" + ); + let base_bytes = std::fs::read(&base).unwrap(); + let (refused, run) = edit_file( + &base, + &dir, + "move-section-before", + json!([{"op":"move_para","address":{"at":{"section":0,"paragraph":2}}, + "to":{"address":{"at":{"section":0,"paragraph":0}},"position":"before"}}]), + ); + assert!( + !run.status.success(), + "a move in front of the section definition owner must refuse" + ); + let diagnostic = String::from_utf8_lossy(&run.stderr).into_owned(); + assert!( + diagnostic.contains("구역") && diagnostic.contains("앞"), + "move refusal must explain the boundary: {diagnostic}" + ); + assert!( + std::fs::read(&base).unwrap() == base_bytes && !refused.exists(), + "the move refusal must leave the source and destination untouched" + ); + + let (output, run) = edit_file( + &base, + &dir, + "move-section-behind", + json!([{"op":"move_para","address":{"at":{"section":0,"paragraph":1}}, + "to":{"address":{"at":{"section":0,"paragraph":2}},"position":"after"}}]), + ); + success( + run, + "move to a position behind the section definition owner", + ); + let after = read(&output); + assert!( + after.sections[0].paragraphs[0] + .controls + .iter() + .any(|control| matches!(control, Control::SectionDef(_))), + "the owner must stay first after a permitted move" + ); + assert_eq!( + index(&after, ANCHOR), + 2, + "the permitted move must have happened behind the owner" + ); + } +} + fn ordered_custom_case(label: &str, marker: &str, start: u32, template: &str) { for extension in ["hwp", "hwpx"] { let fixture = Fixture::new(&format!("ordered-{label}"), extension); diff --git a/crates/hwp-convert/src/lib.rs b/crates/hwp-convert/src/lib.rs index 681050e9..aa457449 100644 --- a/crates/hwp-convert/src/lib.rs +++ b/crates/hwp-convert/src/lib.rs @@ -1,4 +1,4 @@ -//! IR ↔ markdown/JSON 변환. +//! IR <-> markdown/JSON conversion. pub mod address; pub mod base64; @@ -84,17 +84,18 @@ pub use segment_id::{ table_id, }; pub use structure::{ - delete_paragraph, delete_paragraph_at, insert_paragraph, insert_paragraph_at, move_paragraph, - text_in_unwritable_object, + delete_paragraph, delete_paragraph_at, insert_paragraph, insert_paragraph_at, + insert_paragraph_checked, move_paragraph, text_in_unwritable_object, }; pub use style::{display_width, style_table, style_tables}; pub use styles::{NamedStyle, StyleKind, StyleTableTooLarge, list_named_styles}; -/// IR 전체를 JSON으로 직렬화 (구조 검사·디버깅·기계 소비용). +/// Serialize the whole IR as JSON (for structural inspection, debugging and machine consumers). /// -/// `embed_bin`이 참이면 첨부 바이너리(`bin_streams[].data`, 기본 JSON 제외)를 -/// `data_b64` 필드에 base64로 실어 **자급식 JSON**을 만든다 — [`from_json`]이 -/// 다시 읽어 이미지까지 무손실 왕복한다. 거짓이면 이미지 바이트는 빠진다. +/// With `embed_bin`, attached binaries (`bin_streams[].data`, excluded from the default JSON) are +/// carried as base64 in the `data_b64` field, producing a **self-contained JSON** that +/// [`from_json`] reads back for a lossless round trip including images. Without it the image bytes +/// are omitted. pub fn to_json(doc: &Document, pretty: bool, embed_bin: bool) -> serde_json::Result { if !embed_bin { return if pretty { @@ -124,14 +125,14 @@ pub fn to_json(doc: &Document, pretty: bool, embed_bin: bool) -> serde_json::Res } } -/// JSON IR을 문서로 역직렬화 ([`to_json`]의 짝). +/// Deserialize a JSON IR into a document (the counterpart of [`to_json`]). /// -/// `bin_streams[].data_b64`(있을 때)를 디코드해 첨부 바이너리를 복원한다. -/// `data`는 `#[serde(skip)]`이라 base64 필드가 없으면 이미지 바이트는 비어 있다. +/// Decodes `bin_streams[].data_b64` (when present) to restore the attached binaries. `data` is +/// `#[serde(skip)]`, so without the base64 field the image bytes stay empty. pub fn from_json(json: &str) -> Result { let mut value: serde_json::Value = serde_json::from_str(json).map_err(|e| e.to_string())?; - // 역직렬화 전에 data_b64를 분리해 둔다 (Document에는 없는 필드). + // Detach data_b64 before deserializing (Document has no such field). let mut decoded: Vec<(usize, Vec)> = Vec::new(); if let Some(arr) = value.get_mut("bin_streams").and_then(|v| v.as_array_mut()) { for (i, item) in arr.iter_mut().enumerate() { @@ -168,7 +169,7 @@ mod tests { let doc = 표본_문서(); let json = to_json(&doc, true, false).unwrap(); let back = from_json(&json).unwrap(); - // 이미지 바이트 외 IR 전체가 동일해야 한다. + // The whole IR except the image bytes must be identical. assert_eq!(doc, back); } @@ -180,12 +181,12 @@ mod tests { data: vec![0x89, b'P', b'N', b'G', 1, 2, 3, 255, 0, 42], }); - // embed 없이는 이미지 바이트가 빠진다. + // Without embed the image bytes are dropped. let lean = from_json(&to_json(&doc, false, false).unwrap()).unwrap(); assert_eq!(lean.bin_streams[0].name, "BIN0001.png"); assert!(lean.bin_streams[0].data.is_empty()); - // embed면 바이트까지 무손실 왕복. + // With embed the round trip is lossless down to the bytes. let full = from_json(&to_json(&doc, false, true).unwrap()).unwrap(); assert_eq!(full, doc); } diff --git a/crates/hwp-convert/src/structure.rs b/crates/hwp-convert/src/structure.rs index 6703484b..30b1714e 100644 --- a/crates/hwp-convert/src/structure.rs +++ b/crates/hwp-convert/src/structure.rs @@ -168,6 +168,14 @@ pub fn text_in_unwritable_object(doc: &Document, text: &str) -> bool { .any(|s| in_list(&s.paragraphs, text, false)) } +/// Cause and remedy shared by every entry point that would place another paragraph in front of a +/// section's setup paragraph, so `insert_blocks`, addressed `insert_para`, anchored `insert_para` +/// and `move_para` explain the same boundary the same way. +const SECTION_OWNER_INSERT_REFUSAL: &str = "구역 정의 문단 앞에는 삽입할 수 없습니다. insert_blocks는 position=after, insert_para는 before=false로 이 문단 뒤에 삽입하세요"; + +/// Cause and remedy for a move that would land in front of a section's setup paragraph. +const SECTION_OWNER_MOVE_REFUSAL: &str = "구역 정의 문단 앞으로는 이동할 수 없습니다. 이동 대상 위치를 구역 정의 문단 뒤(본문 인덱스 1 이상)로 지정하세요"; + /// Insert a text paragraph after or before the first paragraph containing anchor; return whether inserted. /// The new paragraph inherits the anchor's character/paragraph shapes. Search body, /// table-cell, nested-table and caption paragraphs. @@ -177,10 +185,26 @@ pub fn text_in_unwritable_object(doc: &Document, text: &str) -> bool { /// preorder walk would let a section-0 cell beat a section-1 body, silently changing /// which paragraph a previously working call selects. pub fn insert_paragraph(doc: &mut Document, anchor: &str, text: &str, before: bool) -> bool { - // Both stages share the closure, so keep completion state in Cell and capture only &done. + insert_paragraph_checked(doc, anchor, text, before).unwrap_or(false) +} + +/// [`insert_paragraph`] with the one refusal that a `bool` cannot express: `before` on a paragraph +/// owning `Control::SectionDef` would make it stop being the section's setup paragraph, which the +/// native writers cannot repair (HWPX would inject a second, default `secPr`; HWP5 would leave the +/// section without secd/cold). Returns `Ok(false)` when no anchor matched, exactly like the `bool` +/// form, and refuses before any mutation. +pub fn insert_paragraph_checked( + doc: &mut Document, + anchor: &str, + text: &str, + before: bool, +) -> Result { + // Both stages share the closure, so keep completion and refusal state in Cell and capture only + // &done and &refused. let done = std::cell::Cell::new(false); + let refused = std::cell::Cell::new(false); let mut visit = |list: &mut Vec| -> bool { - if done.get() { + if done.get() || refused.get() { return false; } let Some(i) = list @@ -189,6 +213,15 @@ pub fn insert_paragraph(doc: &mut Document, anchor: &str, text: &str, before: bo else { return false; }; + if before + && list[i] + .controls + .iter() + .any(|control| matches!(control, Control::SectionDef(_))) + { + refused.set(true); + return false; + } let (ps, sty, cs) = para_template(&list[i]); let new = make_paragraph(text, ps, sty, cs); let at = if before { i } else { i + 1 }; @@ -199,17 +232,23 @@ pub fn insert_paragraph(doc: &mut Document, anchor: &str, text: &str, before: bo }; for section in &mut doc.sections { visit(&mut section.paragraphs); + if refused.get() { + return Err(SECTION_OWNER_INSERT_REFUSAL.to_string()); + } if done.get() { - return true; + return Ok(true); } } for section in &mut doc.sections { walk_nested_para_lists(&mut section.paragraphs, &mut visit); + if refused.get() { + return Err(SECTION_OWNER_INSERT_REFUSAL.to_string()); + } if done.get() { - return true; + return Ok(true); } } - false + Ok(false) } /// Delete matching paragraphs from body, cell, nested-table and caption lists. Retain @@ -397,6 +436,19 @@ pub fn move_paragraph( let dst_len = { let (dst_list, _) = list_at_mut(doc, to_list) .ok_or_else(|| "이동 대상 위치를 찾을 수 없습니다".to_string())?; + // A section body list's first paragraph owns the section/column setup, so landing at index + // 0 would displace it exactly as an insert-before does (#451). Nested lists (cells, + // captions) carry no section setup and stay allowed. + if to_index == 0 + && to_list.indices.len() == 1 + && dst_list.first().is_some_and(|p| { + p.controls + .iter() + .any(|c| matches!(c, Control::SectionDef(_))) + }) + { + return Err(SECTION_OWNER_MOVE_REFUSAL.to_string()); + } dst_list.len() }; let effective_dst_len = if same_list { dst_len - 1 } else { dst_len }; @@ -480,7 +532,7 @@ pub fn insert_paragraphs_at( .iter() .any(|control| matches!(control, Control::SectionDef(_))) { - return Err("구역 정의 문단 앞에는 삽입할 수 없습니다. insert_blocks는 position=after, insert_para는 before=false로 이 문단 뒤에 삽입하세요".into()); + return Err(SECTION_OWNER_INSERT_REFUSAL.into()); } let at_idx = if before { idx } else { idx + 1 }; list.splice(at_idx..at_idx, paragraphs); @@ -824,21 +876,52 @@ mod tests { section: 0, indices: vec![0], }; - move_paragraph(&mut doc, &from, &to_list, 0).unwrap(); + move_paragraph(&mut doc, &from, &to_list, 1).unwrap(); let n: usize = doc.sections.iter().map(|s| s.paragraphs.len()).sum(); assert_eq!(n, 3, "길이 불변"); - let moved = &doc.sections[0].paragraphs[0]; + // Index 0 belongs to the section definition owner, so the move lands behind it (#451); + // what this test pins is that the moved paragraph keeps its identity, not its depth. + let moved = &doc.sections[0].paragraphs[1]; assert!( moved.plain_text().contains("셋째"), - "맨 앞으로 이동: {:?}", + "구역정의 뒤로 이동: {:?}", moved.plain_text() ); assert_eq!(moved.header.instance_id, 777, "instance_id 불변(복제 아님)"); } + #[test] + fn move_paragraph_구역정의_앞으로는_거부() { + let mut doc = from_markdown("첫 문단\n\n둘째 문단\n\n셋째 문단"); + let from = SegmentPath { + section: 0, + indices: vec![2], + }; + let to_list = SegmentPath { + section: 0, + indices: vec![0], + }; + let refusal = move_paragraph(&mut doc, &from, &to_list, 0) + .expect_err("구역정의 문단 앞으로의 이동은 거부되어야 함"); + assert!( + refusal.contains("구역") && refusal.contains("앞"), + "거부 사유와 경계를 설명해야 함: {refusal}" + ); + let texts: Vec<_> = doc.sections[0] + .paragraphs + .iter() + .map(|p| p.plain_text()) + .collect(); + assert_eq!( + texts, + vec!["첫 문단", "둘째 문단", "셋째 문단"], + "거부는 어떤 문단도 움직이지 않아야 함" + ); + } + #[test] fn move_paragraph_앵커_컨트롤도_함께_이동() { - let mut doc = from_markdown("본문\n\n| 가 | 나 |\n|----|----|\n| 1 | 2 |\n\n마지막 문단"); + let mut doc = from_markdown("본문\n\n마지막 문단\n\n| 가 | 나 |\n|----|----|\n| 1 | 2 |\n"); let table_para_idx = doc.sections[0] .paragraphs .iter() @@ -852,8 +935,8 @@ mod tests { section: 0, indices: vec![0], }; - move_paragraph(&mut doc, &from, &to_list, 0).unwrap(); - let moved = &doc.sections[0].paragraphs[0]; + move_paragraph(&mut doc, &from, &to_list, 1).unwrap(); + let moved = &doc.sections[0].paragraphs[1]; assert!( moved .controls diff --git a/fixtures/edit-ops/tracer.json b/fixtures/edit-ops/tracer.json index 870c46f7..b04999dd 100644 --- a/fixtures/edit-ops/tracer.json +++ b/fixtures/edit-ops/tracer.json @@ -13,7 +13,7 @@ }, { "op": "insert_para", - "anchor": "generated edit ops tracer fixture", + "anchor": "tracer tail paragraph", "text": "inserted before the anchor: tracer", "before": true }