Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Dec 5, 2025

We were too conservative by defaulting ignorebrackedpastemode in xterm.js to true. it should have been set to false. this solves issues with pasting multi-line strings into zsh, claude code, etc.

add a new context menu option to more easily disable if you're in a weird ssh session or a REPL that doesn't support bracketedpaste.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

Walkthrough

These changes implement reactive bracketed paste mode management across the frontend and backend. The backend adds a terminal escape sequence (\x1b[?2004l) to disable bracketed paste mode during terminal state reset. The frontend centralizes bracketed paste configuration in TermViewModel using a jotai atom initialized from override config, subscribing to changes to update the terminal's ignoreBracketedPasteMode accordingly. A new settings menu option is added with Default, On, and Off choices. The term.tsx component refactors to use the model-provided atom instead of local override config, with the default value changed from false to true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • term-model.ts: Verify proper atom subscription lifecycle—ensure termBPMUnsubFn is correctly assigned and called in dispose to prevent memory leaks. Confirm settings menu generation logic correctly maps Default/On/Off options to atom values.
  • term.tsx: Confirm the default value change from false to true aligns with intended behavior when the atom is undefined.
  • shellcontroller.go: Validate the escape sequence placement occurs before OSC 16162, and confirm no error handling changes are needed for the buffer write.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main changes: enabling bracketed paste mode by default and adding a context menu option for controlling this feature.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for defaulting bracketed paste mode to ON and the addition of a context menu option.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/bpmode

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4449895 and c4a3a05.

📒 Files selected for processing (3)
  • frontend/app/view/term/term-model.ts (6 hunks)
  • frontend/app/view/term/term.tsx (1 hunks)
  • pkg/blockcontroller/shellcontroller.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build for TestDriver.ai
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
pkg/blockcontroller/shellcontroller.go (1)

207-207: LGTM! Standard escape sequence for disabling bracketed paste mode.

The addition of \x1b[?2004l correctly disables bracketed paste mode during terminal state reset, complementing the other terminal reset escape sequences.

frontend/app/view/term/term-model.ts (5)

58-58: LGTM! Field declarations are well-structured.

The new fields termBPMAtom and termBPMUnsubFn properly support the reactive bracketed paste mode functionality.

Also applies to: 64-64


209-209: LGTM! Consistent with existing configuration patterns.

Initialization of termBPMAtom follows the same pattern as other terminal configuration atoms like termThemeNameAtom.


319-324: LGTM! Reactive subscription correctly updates terminal options.

The subscription properly:

  • Checks for terminal existence before updating
  • Uses the negation of allowBPM for ignoreBracketedPasteMode (default trueignoreBracketedPasteMode becomes false)
  • Updates terminal options dynamically without requiring terminal recreation

459-461: LGTM! Proper subscription cleanup prevents memory leaks.

The cleanup in dispose() correctly unsubscribes from the atom, following the same pattern as shellProcStatusUnsubFn.


594-594: LGTM! Settings menu properly implements bracketed paste mode control.

The new "Allow Bracketed Paste Mode" submenu:

  • Follows existing patterns for other terminal settings
  • Provides clear Default/On/Off options
  • Shows the current default value in the Default option label
  • Correctly updates block metadata via SetMetaCommand

Also applies to: 690-728

frontend/app/view/term/term.tsx (1)

263-263: LGTM! Centralized atom and corrected default value.

The change correctly:

  • Uses model.termBPMAtom instead of a local override atom for centralized state management
  • Sets the default to true (consistent with term-model.ts line 321)
  • Results in ignoreBracketedPasteMode defaulting to false (line 279), enabling bracketed paste mode processing by default as intended

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sawka sawka merged commit 0d60950 into main Dec 5, 2025
7 checks passed
@sawka sawka deleted the sawka/bpmode branch December 5, 2025 22:41
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