fix: searching for env and config files in wrong directories#325
fix: searching for env and config files in wrong directories#325pauldelucia wants to merge 2 commits into
Conversation
WalkthroughThe changes update the logic for locating and copying environment and configuration files, now prioritizing the executable's directory before the current working directory. Several function calls are reformatted for improved readability, and minor whitespace cleanups are performed. No public API signatures are changed, and no new features are introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant FileSystem
User->>App: Start application
App->>FileSystem: Check for .env.example or .env in executable dir
alt Found in executable dir
FileSystem-->>App: Return path to found file
App->>FileSystem: Copy file to app data directory
else Not found in executable dir
App->>FileSystem: Check for .env.example or .env in current working dir
alt Found in current working dir
FileSystem-->>App: Return path to found file
App->>FileSystem: Copy file to app data directory
else Not found
FileSystem-->>App: No file found
end
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app_dir.rs (1)
93-125: Excellent fix for the file location logicThis change properly addresses the PR objective by prioritizing the executable directory over the current working directory when searching for environment files. The logic is sound:
- First checks executable directory for
.env.example- Falls back to executable directory for
.env- Then checks current working directory for
.env.example- Finally checks current working directory for
.env(with proper existence validation)This approach is much more reliable for deployed applications where configuration files should be co-located with the executable.
Consider improving error handling consistency:
- fs::copy(&env_example_file_in_exe_dir, env_file_in_app_dir) - .expect("Failed to copy env file"); + fs::copy(&env_example_file_in_exe_dir, &env_file_in_app_dir) + .expect("Failed to copy .env.example file from executable directory");Similar improvements could be made to the other copy operations for more specific error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app_dir.rs(3 hunks)src/backend_task/core/mod.rs(1 hunks)src/backend_task/core/start_dash_qt.rs(1 hunks)src/context.rs(2 hunks)src/ui/identities/add_existing_identity_screen.rs(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/backend_task/core/start_dash_qt.rs (1)
Learnt from: ogabrielides
PR: dashpay/dash-evo-tool#73
File: src/backend_task/core/start_dash_qt.rs:17-18
Timestamp: 2024-11-18T14:40:33.353Z
Learning: In `src/backend_task/core/start_dash_qt.rs`, the `start_dash_qt` function verifies the existence of `dash_qt_path` using `dash_qt_path.exists()`, which the team considers sufficient validation for `custom_dash_qt` provided by the user.
🧬 Code Graph Analysis (1)
src/context.rs (1)
src/app_dir.rs (1)
core_cookie_path(46-60)
🔇 Additional comments (7)
src/backend_task/core/mod.rs (1)
60-63: LGTM: Improved readabilityThe multi-line formatting makes the function call more readable and aligns with similar formatting improvements throughout the codebase.
src/ui/identities/add_existing_identity_screen.rs (1)
192-193: LGTM: Good code hygieneRemoving trailing whitespace improves code quality and consistency. These changes align well with the overall cleanup theme of this PR.
Also applies to: 227-227, 234-234, 247-247, 252-252, 255-255
src/app_dir.rs (2)
1-2: LGTM: Improved import organizationThe import reordering improves code organization and readability.
46-49: LGTM: Better function signature formattingThe multi-line formatting improves readability and is consistent with similar improvements throughout the codebase.
src/context.rs (2)
90-91: LGTM: Improved readabilitySplitting the function call and expect clause across multiple lines improves code readability and makes the error handling more prominent.
110-111: LGTM: Consistent formatting improvementThe multi-line formatting for the expect clause is consistent with the similar change above and improves readability.
src/backend_task/core/start_dash_qt.rs (1)
56-67: LGTM! Improved config file location logic.The updated logic to prioritize the executable's directory over the current working directory is a solid improvement for config file discovery. This approach is more robust, especially when the application is run from different locations or when deployed as a bundled executable.
The error handling with proper fallbacks to the current working directory ensures backward compatibility while making the file location more reliable.
| let env_file_in_app_dir = app_data_dir.join(".env".to_string()); | ||
| if !env_file_in_app_dir.exists() || !env_file_in_app_dir.is_file() { | ||
| // Get the directory where the executable is located | ||
| let exe_dir = std::env::current_exe() |
There was a problem hiding this comment.
I think it would be better to embed the default .env file inside the binary. Looks like include_bytes! can do the job, but I didn't test this.
2 participants
Summary by CodeRabbit
New Features
Style