Carry vt100 patches on a fork branch instead of vendoring#35
Merged
Conversation
Switch vt100 dependency from the in-tree patches/vt100/ copy to a git dep against Junyi-99/vt100-rust on the long-lived `deck` branch. The branch currently carries the same wide-char OOB fix that PATCH.md described. Why: easier upstream sync (normal git merge instead of file copy), patches become shareable (linkable from issues and other projects), and the deck repo loses ~5k lines of vendored source. Trade-off is that a clean build now fetches the fork once (then cargo caches). The decision to keep vt100 vs migrating to alacritty_terminal / wezterm-term is unchanged — see docs/ansi-parser-decision.md, with an update note at the top pointing to the new layout. docs/vt100-fork.md is the canonical reference for what's patched and how to add new fixes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves the patched vt100 dependency out of the repository and into a pinned git fork branch, keeping deck’s PTY screen-grid rendering dependency while removing the vendored source tree.
Changes:
- Replaces the local
patches/vt100override with a git[patch.crates-io]dependency. - Updates
Cargo.lockto pin the fork commit. - Removes the vendored vt100 crate and adds/updates maintenance documentation.
Reviewed changes
Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.toml |
Switches vt100 patch source from local path to git branch. |
Cargo.lock |
Pins vt100 to the fork commit. |
CLAUDE.md |
Updates vt100 wiring guidance. |
docs/ansi-parser-decision.md |
Adds note that vt100 patches moved from vendored source to fork. |
docs/vt100-fork.md |
Adds fork maintenance documentation. |
patches/vt100/.cargo-ok |
Removes vendored crate metadata. |
patches/vt100/.cargo_vcs_info.json |
Removes vendored crate metadata. |
patches/vt100/Cargo.lock |
Removes vendored crate lockfile. |
patches/vt100/Cargo.toml |
Removes vendored crate manifest. |
patches/vt100/Cargo.toml.orig |
Removes vendored crate original manifest. |
patches/vt100/CHANGELOG.md |
Removes vendored crate changelog. |
patches/vt100/LICENSE |
Removes vendored crate license copy. |
patches/vt100/PATCH.md |
Removes local patch notes. |
patches/vt100/README.md |
Removes vendored crate README. |
patches/vt100/src/attrs.rs |
Removes vendored vt100 source. |
patches/vt100/src/callbacks.rs |
Removes vendored vt100 source. |
patches/vt100/src/cell.rs |
Removes vendored vt100 source. |
patches/vt100/src/grid.rs |
Removes vendored vt100 source. |
patches/vt100/src/lib.rs |
Removes vendored vt100 source. |
patches/vt100/src/parser.rs |
Removes vendored vt100 source. |
patches/vt100/src/perform.rs |
Removes vendored vt100 source. |
patches/vt100/src/row.rs |
Removes vendored vt100 source. |
patches/vt100/src/screen.rs |
Removes vendored vt100 source. |
patches/vt100/src/term.rs |
Removes vendored vt100 source. |
Files not reviewed (1)
- patches/vt100/Cargo.toml.orig: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| | Patch | Fixes | Upstream status | | ||
| |---|---|---| | ||
| | `Row::clear_wide` bounds check + `Row::resize` clears truncated wide cells | OOB panic when shrinking a row through a wide character's continuation cell, then erasing the line. See `docs/bugs/2026-05-18-session-switch-residue.md` for how we found it. | [doy/vt100-rust#28](https://github.com/doy/vt100-rust/issues/28) reported, [PR #30](https://github.com/doy/vt100-rust/pull/30) open. | |
Comment on lines
+33
to
+47
| 1. In the fork (`Junyi-99/vt100-rust`), branch off `deck`: | ||
| ```bash | ||
| git clone https://github.com/Junyi-99/vt100-rust | ||
| cd vt100-rust && git checkout deck && git checkout -b fix/<name> | ||
| ``` | ||
| 2. Make the change. Add a regression test under `tests/` — vt100's test | ||
| style is plain `#[test]` functions driving a `Parser` and asserting on | ||
| `screen()`. See `tests/basic.rs` for examples. | ||
| 3. `cargo test`, commit, push. | ||
| 4. **Open a PR against upstream** (`doy/vt100-rust:main`). Even if | ||
| upstream is slow, the PR is the public record of the patch and the | ||
| path back to an unforked dep if the project ever revives. | ||
| 5. **Merge the same change into our `deck` branch** so deck picks it up. | ||
| Typical flow: `gh pr merge --merge` on a PR opened against `deck`, or | ||
| just fast-forward / cherry-pick. |
Comment on lines
+56
to
+57
| 1. `cd vt100-rust && git fetch upstream && git checkout deck` | ||
| 2. `git merge upstream/main` (or `upstream/v0.16.3` for a tagged release) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switch the
vt100patch frompatches/vt100/(in-tree vendored 0.16.2 + diffs) to a git dependency against the long-liveddeckbranch of Junyi-99/vt100-rust. The same wide-char OOB fix thatPATCH.mddocumented is carried on that branch (and submitted upstream as doy/vt100-rust#30).Changes
Cargo.toml:[patch.crates-io]replacedCargo.lock: pins commit on thedeckbranch (Cargo handles this).patches/vt100/deleted (~5k LOC removed).docs/vt100-fork.md(new): canonical reference for what's patched, how to add a new patch, and how to sync upstream.docs/ansi-parser-decision.md: top-of-doc note pointing at the new layout; the underlying decision (keep vt100, not migrate) is unchanged.CLAUDE.md: updated the one-liner about how vt100 is wired.Trade-offs
patches/vt100/)git merge upstream/mainFor a long-lived patch set, the fork wins. Path back to a plain crates.io version once upstream merges our PR(s): drop the
[patch.crates-io]block.Test plan
cargo buildcleancargo test— 215 passed (same as main; nothing depended on the path-style override)cargo clippy— same single pre-existing warning, no new ones🤖 Generated with Claude Code