feat: Add region-based in-place splicing and force overwrite option for extract - #24
Conversation
The changes in this PR will be included in the next version bump.
|
❌ ESLint Check FailedClick to see details
|
- extract.test.ts: sort imports, async map callback, disable no-floating-promises file-wide (matches parser/region tests) - add bumpy bump file for the extract feature (minor) - region.ts: single markerPatterns() helper for read/replace, accept block-comment prefixes when a lang is given, add regionMarker() that closes /* */ and <!-- --> markers - extract.ts: drop its divergent getCommentStyle copy and use regionMarker(), fixing /* #region */ markers being duplicated instead of spliced and html markers being written with // prefixes
|
Dropping publishConfig.name means the package publishes as mdcode-ts, so 'npm install mdcode' / 'pnpm dlx mdcode' now resolve the upstream fork rather than this package. README also advertised @mdcode/mdcode and the JSDoc examples @gcm/mdcode, neither of which was ever published. The CLI command stays 'mdcode' (bin name is unchanged).
|
Addresses the PR review on the region-splice/--force branch. `region.replace()` became the code that rewrites real source files on this branch, and every latent weakness in it was newly destructive. region.ts: replace the line-at-a-time state machine with per-language span discovery plus a single application pass. Extents are found once per distinct language, so cost scales with languages involved rather than regions. The splice now refuses, returning the source byte-identical, when a region is never closed, is closed by a marker naming another region, is closed while a region opened inside it is still open, appears more than once, nests inside one of the same name, or overlaps another requested region. Previously each of these truncated the file to EOF or doubled a body while reporting success. Inserted bodies are re-indented to their marker and take its line ending. Comment styles are modelled as open/close pairs and cover shell, SQL, CSS, semicolon and markup languages, so markers are no longer written as `//` into a zsh script. Region names are validated before interpolation. extract.ts: group targets by resolved path so aliased `file=` spellings are one group and one write (issue #21), confine writes to realpath(--dir), refuse final-component symlinks, refuse targets that are not valid UTF-8, narrow the existence probe to ENOENT, refuse groups mixing region= with whole-file blocks, skip outline=true blocks, and write via temp file + rename with the target's permission bits preserved. cli.ts: report skipped files even under --quiet and exit 2, so `extract -q` can no longer write nothing and look green in CI. ExtractOptions/ExtractResult are exported and the commander options bag is typed. update.ts: respect read()'s found flag; without it an unterminated or missing region silently emptied the markdown block. Docs: document skip-by-default and --force, retire the "100% CLI compatibility" and "exit codes match" claims, make the three worked recipes force-explicit, fix the README block name collision, correct TESTING.md and CLAUDE.md, and fix test:watch (`node -w` is not a valid flag).
|
…dary Follow-ups found while reviewing the previous commit. extract: several blocks each claiming to be the whole of one file are only compatible if byte-identical. Previously the first one won and the rest were discarded silently; now a disagreement is refused and reported, matching how mixed region/whole-file groups are already handled. Identical duplicates still write, since there is nothing to disagree about. Remove ExtractOptions.sourcePath: it was accepted, passed by the CLI, and never read. It became visible when the type was exported, so drop it rather than publish a knob that does nothing. Document the inode tradeoff on writeAtomic: rename() replaces the file, so hard links keep the old contents and xattrs/ACLs are not carried over. Accepted deliberately, and it is also why the splice path refuses symlinks outright rather than relying on rename's behaviour. docs: state that `update` honours any path a block's `file=` names, including paths outside the markdown's directory, because that path is an explicit instruction from the markdown's author. Record the consequence — update will inline any file the process can read — and the resulting rule: treat untrusted markdown like untrusted code. The asymmetry with extract, which is confined to --dir, is now written down rather than implied.
|
|
adrianbrowning
commented
Sep 8, 2026
📖 How to use this review
Click any section to expand/collapse details. Produced by 9 domain reviewers (security, bug, thermo/maintainability, holistic, perf, TypeScript, testing, devops, scope) whose findings were then re-verified against the working tree by 2 independent validators; 17 unsubstantiated findings were dropped. 🎯 Executive SummaryStatus: ❌ Changes Required 📋 Quick Stats
The engineering here is genuinely careful — per-line EOL preservation, canonical path grouping, an explicit refusal catalogue, atomic writes with mode preservation, and unusually strong tests. Two things block merge: the new path-containment rule refuses the exact 🔴 Critical Issues (Must Fix Before Merge)Issue 1: New |
No description provided.