feat: add tmux clipboard yank bindings and LSP packages - #795
Conversation
- Replace capture-pane file save with clipboard copy via pbcopy - Add Ctrl+B y for full pane history and Ctrl+B v for visible pane - Add isLSP flag to lib/host.nix for conditional LSP installation - Add gopls, lua-language-server, nil, typescript-language-server, pyright Entire-Checkpoint: e6097874141f
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the development environment by improving tmux's clipboard integration and introducing a flexible mechanism for installing Language Server Protocol tools. These changes allow for more efficient text manipulation within tmux and provide a configurable way to manage development tooling based on host-specific needs. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR introduces a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Mesa DescriptionTL;DRReplaced tmux file-save capture with clipboard yank bindings and introduced a configurable What changed?
Description generated by Mesa. Update settings |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Entire-Checkpoint: 3a40568b76ee
There was a problem hiding this comment.
Code Review
This pull request introduces new tmux bindings for copying pane content to the clipboard and adds conditional installation of LSP packages. The changes are well-structured. I've suggested an improvement to the tmux configuration to make the new clipboard bindings cross-platform, working on both macOS and Linux, which aligns with the apparent goal of portability in your dotfiles.
| bind y run-shell 'tmux capture-pane -pS - | pbcopy && tmux display-message "Full pane history copied to clipboard"' | ||
| bind v run-shell 'tmux capture-pane -p | pbcopy && tmux display-message "Visible pane copied to clipboard"' |
There was a problem hiding this comment.
The use of pbcopy makes these bindings specific to macOS. To make your configuration more portable and work on Linux systems (both X11 and Wayland), you could use a shell fallback mechanism. Your Nix configuration already installs xclip and wl-clipboard for Linux. This change will attempt to use pbcopy first (for macOS), then wl-copy (for Wayland), and finally xclip (for X11).
bind y run-shell 'tmux capture-pane -pS - | (pbcopy || wl-copy || xclip -in -selection clipboard) && tmux display-message "Full pane history copied to clipboard"'
bind v run-shell 'tmux capture-pane -p | (pbcopy || wl-copy || xclip -in -selection clipboard) && tmux display-message "Visible pane copied to clipboard"'
There was a problem hiding this comment.
Pull request overview
This PR adds tmux clipboard yank bindings and introduces conditional LSP package installation. It replaces the previous file-save capture binding with clipboard-based commands and adds a feature flag system for managing LSP package installation across different hosts.
Changes:
- Added
isLSPflag to enable/disable LSP package installation per-host - Replaced tmux file-save pane capture with clipboard yank bindings for full and visible pane content
- Added 5 LSP packages (gopls, lua-language-server, nil, typescript-language-server, pyright) with conditional installation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/host.nix | Adds isLSP boolean flag (defaults to true) for controlling LSP package installation |
| home-manager/programs/tmux/tmux.conf | Replaces Ctrl+B P file-save binding with Ctrl+B y (full pane) and Ctrl+B v (visible pane) clipboard yank bindings using pbcopy |
| home-manager/packages/default.nix | Imports isLSP flag and adds conditional LSP package list using lib.optionals |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bind y run-shell 'tmux capture-pane -pS - | pbcopy && tmux display-message "Full pane history copied to clipboard"' | ||
| bind v run-shell 'tmux capture-pane -p | pbcopy && tmux display-message "Visible pane copied to clipboard"' |
There was a problem hiding this comment.
The pbcopy command is macOS-specific and will fail on Linux systems. The codebase already has xclip installed on Linux (see home-manager/packages/default.nix line 126), and the tmux yank plugin is already configured. Consider using platform-conditional logic or relying on the tmux yank plugin which handles cross-platform clipboard operations automatically. Alternatively, use a shell conditional like command -v pbcopy >/dev/null && pbcopy || xclip -selection clipboard to handle both platforms.
| bind y run-shell 'tmux capture-pane -pS - | pbcopy && tmux display-message "Full pane history copied to clipboard"' | |
| bind v run-shell 'tmux capture-pane -p | pbcopy && tmux display-message "Visible pane copied to clipboard"' | |
| bind y run-shell 'tmux capture-pane -pS - | sh -c "command -v pbcopy >/dev/null 2>&1 && pbcopy || xclip -selection clipboard" && tmux display-message "Full pane history copied to clipboard"' | |
| bind v run-shell 'tmux capture-pane -p | sh -c "command -v pbcopy >/dev/null 2>&1 && pbcopy || xclip -selection clipboard" && tmux display-message "Visible pane copied to clipboard"' |
| isDesktop = false; | ||
|
|
||
| # Install language server packages | ||
| isDev = true; |
There was a problem hiding this comment.
The new isLSP flag in lib/host.nix lacks test coverage. Following the established pattern seen with isDesktop and other host flags in tests/lib.nix (lines 54-58), a test should be added to validate that isLSP exists and is a boolean. This ensures consistency with the testing conventions used for other configuration flags in lib/host.nix.
| isDev = true; | |
| isLSP = true; | |
| isDev = isLSP; |
Changes
Ctrl+B Pfile-save capture with clipboard-based yank bindingsCtrl+B yto copy full pane history to clipboardCtrl+B vto copy visible pane to clipboardisLSPflag inlib/host.nixfor conditional LSP package installationTechnical Details
run-shellwithcapture-pane -p | pbcopyto avoid the "Can't write to client" error from chained tmux commandsisLSPfollows the same pattern asisDesktop— defaults totrue, can be overridden per-hostrust-analyzersincerustupalready provides itTesting
capture-pane -p | pbcopypipeline works end-to-endGenerated with Claude Code
Summary by cubic
Switch tmux from saving pane output to a file to copying to the clipboard, and add a per-host isDev toggle for installing LSP packages. This makes copying output easier and lets hosts opt into language servers.
Written for commit 78836f2. Summary will update on new commits.