Skip to content

Fix cmd-click crash when command-mark data is unavailable#695

Closed
onemedicine wants to merge 2 commits into
gnachman:masterfrom
onemedicine:onemedicine-command-click-crash
Closed

Fix cmd-click crash when command-mark data is unavailable#695
onemedicine wants to merge 2 commits into
gnachman:masterfrom
onemedicine:onemedicine-command-click-crash

Conversation

@onemedicine

Copy link
Copy Markdown
Contributor

Summary

  • guard VT100ScreenState command-mark lookup against out-of-bounds coordinates, missing marks, and missing interval-tree entries
  • let command-click URL resolution fall through cleanly when command-mark data is unavailable instead of crashing
  • add targeted VT100ScreenTests coverage for the out-of-bounds and no-mark cases, and document the fix in the 3.7 release notes

Test plan

  • make paranoid-deps
  • tools/run_tests.expect ModernTests/VT100ScreenTests/testCommandMarkAtReturnsNilWhenLineIsOutOfBounds ModernTests/VT100ScreenTests/testCommandMarkAtReturnsNilWhenNoMarkOnLine

Notes

  • I rebased this to the current master so the PR is a single fix commit on top of upstream.

Copilot AI review requested due to automatic review settings July 1, 2026 16:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens command-mark lookup in VT100ScreenState so Cmd-click URL/prompt resolution can gracefully fall back when command-mark data is missing or inconsistent, avoiding a crash in the terminal interaction flow.

Changes:

  • Add bounds/validity guards in VT100ScreenState -commandMarkAt:mustHaveCommand:range: for out-of-range coordinates, missing marks, and missing interval-tree entries.
  • Add targeted VT100ScreenTests coverage for out-of-bounds and no-mark cases.
  • Document the crash fix in the 3.7 release notes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
sources/VT100Screen/VT100ScreenState.m Adds defensive checks so command-mark lookup returns nil instead of crashing when mark data is unavailable/invalid.
ModernTests/VT100ScreenTests.swift Adds regression tests covering out-of-bounds and no-mark command-mark lookups.
docs/notes-3.7.txt Notes the Cmd-click crash fix in the 3.7 bug fixes section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ModernTests/VT100ScreenTests.swift
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gnachman

Copy link
Copy Markdown
Owner

Thank you! This is a clean, well-guarded fix, and I appreciate the thorough regression tests covering the out-of-bounds, no-mark, and mark-without-command paths (and for addressing the review note about mustHaveCommand). All three tests pass here. Merged (squashed) as f036dee, with you as the author.

@gnachman gnachman closed this Jul 24, 2026
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.

3 participants