feat (cli): implementing environment variable handling #652
Conversation
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
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| Ok(vars) | ||
| } No newline at end of file |
There was a problem hiding this comment.
style: missing newline at end of file
Prompt To Fix With AI
This is a comment left during a code review.
Path: helix-cli/src/env_utils.rs
Line: 70:70
Comment:
**style:** missing newline at end of file
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+42
to
+47
| let value = if (value.starts_with('"') && value.ends_with('"')) | ||
| || (value.starts_with('\'') && value.ends_with('\'')) { | ||
| &value[1..value.len() - 1] | ||
| } else { | ||
| value | ||
| }; |
There was a problem hiding this comment.
logic: quote removal logic could fail on single-character quoted strings due to potential underflow in value.len() - 1
Suggested change
| let value = if (value.starts_with('"') && value.ends_with('"')) | |
| || (value.starts_with('\'') && value.ends_with('\'')) { | |
| &value[1..value.len() - 1] | |
| } else { | |
| value | |
| }; | |
| let value = if value.len() >= 2 && ((value.starts_with('"') && value.ends_with('"')) | |
| || (value.starts_with('\'') && value.ends_with('\''))) { | |
| &value[1..value.len() - 1] | |
| } else { | |
| value | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: helix-cli/src/env_utils.rs
Line: 42:47
Comment:
**logic:** quote removal logic could fail on single-character quoted strings due to potential underflow in `value.len() - 1`
```suggestion
let value = if value.len() >= 2 && ((value.starts_with('"') && value.ends_with('"'))
|| (value.starts_with('\'') && value.ends_with('\''))) {
&value[1..value.len() - 1]
} else {
value
};
```
How can I resolve this? If you propose a fix, please make it concise.
helix-cli/Cargo.toml
Outdated
|
|
||
| [[bin]] | ||
| name = "helix_test_env_parser" | ||
| path = "src/test_env_parser.rs" |
There was a problem hiding this comment.
style: Missing newline at end of file
Prompt To Fix With AI
This is a comment left during a code review.
Path: helix-cli/Cargo.toml
Line: 40:40
Comment:
**style:** Missing newline at end of file
How can I resolve this? If you propose a fix, please make it concise.
xav-db
changed the title
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.
Description
Related Issues
Closes #
Checklist when merging to main
rustfmthelix-cli/Cargo.tomlandhelixdb/Cargo.tomlAdditional Notes
Greptile Overview
Updated On: 2025-10-08 17:37:33 UTC
Summary
This PR implements environment variable handling for the Helix CLI, enabling users to specify custom `.env` files for their local instances. The feature adds comprehensive support for loading environment variables from both `.env` files and the shell environment, with proper precedence handling where shell variables take priority over file-based ones.The implementation adds a new
env_utilsmodule that handles.envfile parsing with support for quoted values, comments, and validation. TheLocalInstanceConfigstruct now includes an optionalenv_filefield that allows users to specify custom environment files in theirhelix.tomlconfiguration. The Docker integration has been enhanced to dynamically load these user-defined environment variables while filtering outHELIX_prefixed variables to prevent conflicts with internal system variables.Key components of the implementation:
load_env_variablesfunction inenv_utils.rsprovides robust parsing of.envfiles with proper error handling and user feedbackenv_filefield is seamlessly integrated into the existing configuration system with proper serde defaultsDockerManagernow dynamically generates environment variables for containers, combining built-in Helix variables with user-defined onesThis change fits well with the existing codebase architecture, following established patterns for configuration management and Docker integration. The modular design keeps environment handling separate while integrating cleanly with existing systems.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|----------|-------|
| helix-cli/Cargo.toml | 4/5 | Removed trailing newline - goes against standard conventions that files should end with newlines |
| helix-cli/src/commands/add.rs | 5/5 | Added proper initialization of env_file field to None for new local instances |
| helix-cli/src/main.rs | 5/5 | Added env_utils module declaration to integrate environment variable handling |
| helix-cli/src/commands/migrate.rs | 5/5 | Added env_file field initialization during v1 to v2 project migration |
| helix-cli/src/docker.rs | 4/5 | Enhanced DockerManager with dynamic environment variable loading - potential issues with error propagation |
| helix-cli/src/config.rs | 5/5 | Added optional env_file field to LocalInstanceConfig with proper serde defaults |
| helix-cli/src/commands/integrations/fly.rs | 5/5 | Fixed error handling for environment_variables method call with proper ? operator |
| helix-cli/src/env_utils.rs | 4/5 | New module implementing comprehensive environment variable handling - quote parsing logic could be more robust |
Sequence Diagram
sequenceDiagram participant User participant CLI as Helix CLI participant PM as ProjectManager participant Docker as DockerManager participant EnvUtils as env_utils participant FileSystem as File System User->>CLI: "helix init/add/build/push <instance>" CLI->>PM: "find_and_load(path)" PM->>FileSystem: "read helix.toml" FileSystem-->>PM: "config data" PM-->>CLI: "ProjectContext" CLI->>PM: "get_instance(instance_name)" PM-->>CLI: "InstanceInfo" alt Local Instance with env_file CLI->>Docker: "environment_variables(instance_name)" Docker->>PM: "get_instance(instance_name)" PM-->>Docker: "LocalInstanceConfig" Docker->>EnvUtils: "load_env_variables(env_file, project_root)" EnvUtils->>FileSystem: "read_to_string(env_file)" FileSystem-->>EnvUtils: "env file content" EnvUtils->>EnvUtils: "parse KEY=VALUE pairs" EnvUtils->>EnvUtils: "get shell environment vars" EnvUtils-->>Docker: "HashMap<String, String>" Docker->>Docker: "merge with HELIX_ variables" Docker-->>CLI: "Vec<String> env vars" end CLI->>Docker: "generate_docker_compose(instance, config)" Docker->>Docker: "environment_variables(instance)" Docker-->>CLI: "docker-compose.yml content" CLI->>FileSystem: "write docker-compose.yml" CLI->>Docker: "build_image(instance)" Docker->>FileSystem: "docker compose build"