feat(programs): add language servers and php program - #1051
Conversation
Entire-Checkpoint: ac8a2918ac29
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello, 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 significantly enhances the development environment by introducing comprehensive support for PHP, including its interpreter and a dedicated language server. It also expands code intelligence capabilities across multiple languages by integrating various language servers into their respective program configurations. These changes streamline the setup process for developers working with different programming languages, providing a more robust and efficient coding experience. 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
|
📝 WalkthroughWalkthroughThis PR extends multiple home-manager language runtime modules with corresponding language server and development tool packages. It introduces a new PHP program module and updates the program index to expose it, while also adding language servers to existing Bash, Go, Node, Python, Rust, and Zig modules. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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;DRAdded built-in LSP support for Bash, TypeScript, Python, Go, Rust, and Zig, and introduced a new PHP program with Intelephense, while also registering What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds several language servers for existing languages (Bash, Go, Node/TS, Python, Rust, Zig) and introduces a new program configuration for PHP, including its language server (Intelephense). The changes are well-structured and follow existing patterns in the repository. My main feedback is a suggestion to sort the list of enabled programs in home-manager/programs/default.nix to improve future maintainability.
| node | ||
| php |
There was a problem hiding this comment.
For better maintainability, please sort the list of enabled programs alphabetically. While you've added node and php in the correct alphabetical position relative to their neighbors, the entire list is not sorted (e.g., zig appears before starship).
Keeping this list sorted will make it easier to manage as more programs are added. Here is the suggested sorted list:
[
atuin
bash
bat
btop
delta
direnv
fd
fish
fnm
fzf
gh
ghq
git
go
lazydocker
lazygit
lsd
neovim
node
php
python
rust
ssh
starship
tms
tmux
zig
zoxide
zsh
]There was a problem hiding this comment.
Pull request overview
Adds additional language tooling to the Home Manager program modules, including a new PHP program module and multiple language servers, and wires PHP (and Node) into the global programs import list.
Changes:
- Add
home-manager/programs/phpmodule withphp+nodePackages.intelephense. - Add language servers to several existing program modules (e.g.,
zls,rust-analyzer,pyright,typescript-language-server,bash-language-server,gopls). - Include
php(andnode) inhome-manager/programs/default.nixso they’re imported/enabled with other program modules.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| home-manager/programs/zig/default.nix | Adds zls to Zig packages. |
| home-manager/programs/rust/default.nix | Adds rust-analyzer to Rust packages. |
| home-manager/programs/python/default.nix | Adds pyright to Python packages. |
| home-manager/programs/php/default.nix | New PHP module installing php and intelephense. |
| home-manager/programs/node/default.nix | Adds typescript-language-server to Node packages. |
| home-manager/programs/go/default.nix | Adds gopls to Go packages. |
| home-manager/programs/default.nix | Wires php import + adds node and php to the module list. |
| home-manager/programs/bash/default.nix | Installs bash-language-server via nodePackages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| home.packages = with pkgs; [ | ||
| go | ||
| gopls |
There was a problem hiding this comment.
gopls is now installed unconditionally via the Go program module, but it’s already included in home-manager/packages/default.nix under lib.optionals isDev (lines ~161-167). This creates duplication for dev hosts and also bypasses the existing isDev gating. Consider keeping gopls in only one place (either remove it from the isDev package list or make this module conditional on isDev).
| gopls |
| go | ||
| gopls | ||
| ]; |
There was a problem hiding this comment.
PR description lists added language servers but doesn’t mention gopls; however this change adds it to the Go program module. Please either update the PR description to include gopls or drop this addition if it’s out of scope.
| { | ||
| home.packages = with pkgs; [ | ||
| python3 | ||
| pyright |
There was a problem hiding this comment.
pyright is already included in home-manager/packages/default.nix under lib.optionals isDev (lines ~161-167). Adding it here duplicates the package on dev hosts and makes it install for non-dev hosts too (since programs/* modules are always imported from home-manager/default.nix). Consider keeping pyright in just one location or making this conditional on isDev.
| pyright |
| { | ||
| home.packages = with pkgs; [ | ||
| node | ||
| nodePackages.typescript-language-server |
There was a problem hiding this comment.
nodePackages.typescript-language-server is already included in home-manager/packages/default.nix under lib.optionals isDev (lines ~161-167). Adding it here duplicates it on dev hosts and makes it install on non-dev hosts as well (since the programs modules are imported unconditionally). Consider consolidating to a single source of truth or gating this module on isDev.
| nodePackages.typescript-language-server |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@home-manager/programs/node/default.nix`:
- Around line 3-6: The package list in home.packages uses the incorrect nixpkgs
attribute name "node"; replace "node" with "nodejs" (or a versioned variant like
"nodejs_24") so the attribute resolves correctly while leaving
"nodePackages.typescript-language-server" as-is; update the package entry in the
array where "home.packages" is defined to use "nodejs" instead of "node".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 510aa42e-7b5c-4c20-ba59-cbe5681ac493
📒 Files selected for processing (8)
home-manager/programs/bash/default.nixhome-manager/programs/default.nixhome-manager/programs/go/default.nixhome-manager/programs/node/default.nixhome-manager/programs/php/default.nixhome-manager/programs/python/default.nixhome-manager/programs/rust/default.nixhome-manager/programs/zig/default.nix
Entire-Checkpoint: ac571f095112
Summary
home-manager/programs/php/) withphpandnodePackages.intelephense(PHP Intelephense LSP)bash-language-server,typescript-language-server,pyright,rust-analyzer,zlsphpandnodeinto the programs list indefault.nixTest plan
home-manager switchto verify all packages build and install correctly$PATHafter switch🤖 Generated with Claude Code
Summary by cubic
Add built-in LSP support for Bash, TypeScript, Python, Go, and Zig, and add a PHP program with Intelephense. Also register
phpandnodein the programs list and switch Node tonodejs.New Features
phpandnodePackages.intelephense.nodePackages.bash-language-server,nodePackages.typescript-language-server,pyright,gopls,zls.home-manager/programs/default.nix: importphp; includenodeandphp.Bug Fixes
nodejspackage name for Node.rust-analyzer.Written for commit a1b6220. Summary will update on new commits.