Skip to content

feat: add HWPX fragment paste support#880

Open
dragonnite1221-lgtm wants to merge 1 commit into
edwardkim:develfrom
dragonnite1221-lgtm:public/hwpx-fragment-paste
Open

feat: add HWPX fragment paste support#880
dragonnite1221-lgtm wants to merge 1 commit into
edwardkim:develfrom
dragonnite1221-lgtm:public/hwpx-fragment-paste

Conversation

@dragonnite1221-lgtm
Copy link
Copy Markdown

@dragonnite1221-lgtm dragonnite1221-lgtm commented May 13, 2026

Adds HWPX fragment paste support, WASM bridge APIs, rhwp-studio insertion UI, and regression tests. Internal work logs, local environment paths, and operational notes were intentionally excluded from this public branch.

Architecture note: this PR intentionally keeps two paste entry points for different safety boundaries.

  • pasteHwpxFragment / fragment_paste.rs is the raw XML primitive. It performs byte-preserving section/header edits and owns HWPX ID remapping for raw snippets.
  • pasteHwpxFragmentInDocument / fragment_paste_in_document.rs is the editor-facing bridge. It reuses the raw XML primitive, then reparses the updated section back into DocumentCore so rendering and later edit commands stay in sync.
  • cross_document_migrate.rs is the IR migration primitive for future structured cross-document copy/paste. It is kept separate from the raw HWPX fragment path because it operates on Document/DocInfo IR, not byte-preserved HWPX XML. The convergence point is at the command/API layer: raw HWPX fragments use the byte-preserving path now; structured IR copy/paste can use the migration primitive once the editor exposes that workflow.

Review follow-up in this revision:

  • Rebased onto current origin/devel and preserved the HWPX external-image path fix in parse_hwpx_with_raw_xmls.
  • Removed the as any call by using the generated WASM binding type for pasteHwpxFragmentInDocument.
  • Removed local-machine yangsik smoke tests and made the remaining Document bridge tests use the bundled saved/04-blank_hwpx_empty.hwpx fixture so CI gets real regression coverage.
  • Aligned the blank HWPX seed comment with saved/04-blank_hwpx_empty.hwpx.
  • Removed the unused build_occupied_sets helper.

Validation performed locally:

  • cargo test --test fragment_paste_integration --test fragment_paste_in_document
  • cargo test fragment_paste
  • npm run build in rhwp-studio/

@dragonnite1221-lgtm
Copy link
Copy Markdown
Author

Updated the branch on top of current origin/devel and resolved conflicts in rhwp-studio/index.html, rhwp-studio/vite.config.ts, and src/document_core/mod.rs.

Current validation after rebase:

  • gitleaks detect --source . --log-opts origin/devel..HEAD --redact --no-banner --verbose: no leaks found
  • sensitive string scan on origin/devel..HEAD: no internal paths/IP/token patterns found
  • cargo test fragment_paste --tests: passed
  • docker compose --env-file .env.docker run --rm wasm: passed, local pkg regenerated for verification
  • cd rhwp-studio && npm run build: passed

I also added a local pre-push guard in my working copy to prevent accidentally pushing local/*, archive/internal/*, or public branches containing internal paths such as mydocs/.

@edwardkim
Copy link
Copy Markdown
Owner

Welcome to the rhwp project, @dragonnite1221-lgtm! 🎉 This is your first PR and the scope is impressive — HWPX fragment paste with both raw XML and IR migration paths, a yangsik dialog, and thorough test coverage.

We've completed a detailed code review. The overall quality is good, but there are a few items to address before we can merge.

What we liked

  • Dual approach (raw XML path + IR migration path) with independent tests for each
  • Byte-preserving principle consistently applied
  • Path traversal defense in vite-plugin-yangsik-fragments.ts
  • ~40 unit tests + 4 integration tests — solid coverage

Requested changes

1. Dual implementation overlap (medium)

fragment_paste.rs (raw XML ID remap) and cross_document_migrate.rs (IR ID remap) implement the same purpose with separate IdRemap structs. The WASM bridge exposes two entry points (pasteHwpxFragment + pasteHwpxFragmentInDocument) but the PR description doesn't explain why both are needed or how they'll converge.

Ask: Please add a note in the PR description explaining the intent, or consolidate into one path if the other is experimental.

2. Type cast in wasm-bridge.ts (medium)

(this.doc as any).pasteHwpxFragmentInDocument(...)

This bypasses compile-time type safety. Please add the method to the WASM binding type definition instead of using as any.

3. Environment-dependent tests (medium)

fragment_paste_in_document_yangsik.rs depends on ~/rhwp-layout-profiles/personal-templates/양식_3bf55ee3.hwpx — a path that only exists on your local machine. These tests are always skipped in CI and provide no regression protection.

Ask: Either bundle a small test fixture in saved/ or samples/, or remove the environment-dependent tests and keep only the self-contained ones.

4. Comment/code mismatch (low)

wasm_api.rs comments reference saved/03-blank_hwpx.hwpx but the actual added file is saved/04-blank_hwpx_empty.hwpx. Please align the comment.

5. Unused function (low)

build_occupied_sets in fragment_paste.rs appears to be defined but the same logic is reimplemented inline in recompute_one_table. This may trigger dead_code warnings. Please remove the unused function or use it.

How to proceed

  1. Address items baseline/line_height/line_spacing 역공학 #1cellzoneList(셀 영역 배경) 미지원으로 표 배경 누락 #5 above
  2. Rebase onto current devel to resolve the conflict
  3. Push the updated branch — we'll re-review

We're also happy to discuss architecture decisions (e.g., the dual-path approach) in a GitHub Discussion or Issue if that would help.

Thank you for the contribution, and looking forward to the updated PR!

@dragonnite1221-lgtm dragonnite1221-lgtm force-pushed the public/hwpx-fragment-paste branch from 44ca568 to 29c7cdb Compare May 14, 2026 14:10
@dragonnite1221-lgtm dragonnite1221-lgtm force-pushed the public/hwpx-fragment-paste branch from 29c7cdb to dd430bf Compare May 14, 2026 14:17
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