implemented new changes regarding issue 5 #15
Open
sudarsan1030 wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe PR adds an env-file parser that reads ChangesEnv file parsing
Main output text
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/env_validator.rs`:
- Around line 41-53: In env_validator.rs, update the line parsing logic in the
env-file reader so malformed records are rejected instead of ignored: the
current loop in the env parsing function silently skips non-empty, non-comment
lines without an '=' and accepts entries with an empty key. Add validation
around the existing split_once('=') handling so only non-empty keys are
inserted, and return an error for any malformed line rather than continuing with
Ok(...). Use the existing env-file parsing path and the vars insert logic as the
place to enforce this.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bd12059f-17bb-47c5-bd87-31d3d790138b
📒 Files selected for processing (2)
src/env_validator.rssrc/main.rs
Comment on lines
+41
to
+53
| for line in content.lines() { | ||
| let line = line.trim(); | ||
|
|
||
| if line.is_empty() || line.starts_with('#') { | ||
| continue; | ||
| } | ||
|
|
||
| if let Some((key, value)) = line.split_once('=') { | ||
| vars.insert( | ||
| key.trim().to_string(), | ||
| value.trim().to_string(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject malformed env records instead of skipping them.
Any non-empty, non-comment line without = is silently dropped here, and =value is accepted with an empty key. That turns env-file typos into missing configuration while still returning Ok(...).
Suggested fix
- for line in content.lines() {
+ for (line_no, line) in content.lines().enumerate() {
let line = line.trim();
if line.is_empty() || line.starts_with('#') {
continue;
}
- if let Some((key, value)) = line.split_once('=') {
- vars.insert(
- key.trim().to_string(),
- value.trim().to_string(),
- );
- }
+ let (key, value) = line.split_once('=').ok_or_else(|| {
+ io::Error::new(
+ io::ErrorKind::InvalidData,
+ format!("invalid env entry on line {}: missing '='", line_no + 1),
+ )
+ })?;
+
+ let key = key.trim();
+ if key.is_empty() {
+ return Err(io::Error::new(
+ io::ErrorKind::InvalidData,
+ format!("invalid env entry on line {}: empty key", line_no + 1),
+ ));
+ }
+
+ vars.insert(key.to_string(), value.trim().to_string());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for line in content.lines() { | |
| let line = line.trim(); | |
| if line.is_empty() || line.starts_with('#') { | |
| continue; | |
| } | |
| if let Some((key, value)) = line.split_once('=') { | |
| vars.insert( | |
| key.trim().to_string(), | |
| value.trim().to_string(), | |
| ); | |
| } | |
| for (line_no, line) in content.lines().enumerate() { | |
| let line = line.trim(); | |
| if line.is_empty() || line.starts_with('#') { | |
| continue; | |
| } | |
| let (key, value) = line.split_once('=').ok_or_else(|| { | |
| io::Error::new( | |
| io::ErrorKind::InvalidData, | |
| format!("invalid env entry on line {}: missing '='", line_no + 1), | |
| ) | |
| })?; | |
| let key = key.trim(); | |
| if key.is_empty() { | |
| return Err(io::Error::new( | |
| io::ErrorKind::InvalidData, | |
| format!("invalid env entry on line {}: empty key", line_no + 1), | |
| )); | |
| } | |
| vars.insert(key.to_string(), value.trim().to_string()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/env_validator.rs` around lines 41 - 53, In env_validator.rs, update the
line parsing logic in the env-file reader so malformed records are rejected
instead of ignored: the current loop in the env parsing function silently skips
non-empty, non-comment lines without an '=' and accepts entries with an empty
key. Add validation around the existing split_once('=') handling so only
non-empty keys are inserted, and return an error for any malformed line rather
than continuing with Ok(...). Use the existing env-file parsing path and the
vars insert logic as the place to enforce this.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/env_validator.rs`:
- Around line 73-84: The report output in `env_validator.rs` is currently
iterating `HashMap` entries directly in the “Unused Variables” and “Empty
Variables” sections, which makes the printed order nondeterministic. Update the
report generation logic around the existing `env` iteration to collect the
matching keys first, sort them, and then print them in a stable order before
emitting each section so `println!` output is deterministic and test-friendly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 932b6eea-3ed4-4e1a-b81b-85ebe1f8b3b7
📒 Files selected for processing (2)
src/env_validator.rssrc/main.rs
✅ Files skipped from review due to trivial changes (1)
- src/main.rs
Comment on lines
+73
to
+84
| println!("\nUnused Variables:"); | ||
| for key in env.keys() { | ||
| if !extracted.contains(key) { | ||
| println!("- {}", key); | ||
| } | ||
| } | ||
|
|
||
| println!("\nEmpty Variables:"); | ||
| for (key, value) in env { | ||
| if value.trim().is_empty() { | ||
| println!("- {}", key); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sort report entries before printing.
Lines 74-83 iterate HashMap directly, so the "Unused Variables" and "Empty Variables" sections come out in nondeterministic order. That makes the CLI output noisy and brittle to assert in tests.
Suggested fix
println!("\nUnused Variables:");
- for key in env.keys() {
- if !extracted.contains(key) {
- println!("- {}", key);
- }
- }
+ let mut unused: Vec<_> = env
+ .keys()
+ .filter(|key| !extracted.contains(*key))
+ .cloned()
+ .collect();
+ unused.sort();
+ for key in unused {
+ println!("- {}", key);
+ }
println!("\nEmpty Variables:");
- for (key, value) in env {
- if value.trim().is_empty() {
- println!("- {}", key);
- }
- }
+ let mut empty: Vec<_> = env
+ .iter()
+ .filter(|(_, value)| value.trim().is_empty())
+ .map(|(key, _)| key.clone())
+ .collect();
+ empty.sort();
+ for key in empty {
+ println!("- {}", key);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| println!("\nUnused Variables:"); | |
| for key in env.keys() { | |
| if !extracted.contains(key) { | |
| println!("- {}", key); | |
| } | |
| } | |
| println!("\nEmpty Variables:"); | |
| for (key, value) in env { | |
| if value.trim().is_empty() { | |
| println!("- {}", key); | |
| } | |
| println!("\nUnused Variables:"); | |
| let mut unused: Vec<_> = env | |
| .keys() | |
| .filter(|key| !extracted.contains(*key)) | |
| .cloned() | |
| .collect(); | |
| unused.sort(); | |
| for key in unused { | |
| println!("- {}", key); | |
| } | |
| println!("\nEmpty Variables:"); | |
| let mut empty: Vec<_> = env | |
| .iter() | |
| .filter(|(_, value)| value.trim().is_empty()) | |
| .map(|(key, _)| key.clone()) | |
| .collect(); | |
| empty.sort(); | |
| for key in empty { | |
| println!("- {}", key); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/env_validator.rs` around lines 73 - 84, The report output in
`env_validator.rs` is currently iterating `HashMap` entries directly in the
“Unused Variables” and “Empty Variables” sections, which makes the printed order
nondeterministic. Update the report generation logic around the existing `env`
iteration to collect the matching keys first, sort them, and then print them in
a stable order before emitting each section so `println!` output is
deterministic and test-friendly.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
added parser for .emv kinda files , returns values in for dictionary , ignores comments and blank lines
Summary by CodeRabbit
New Features
Bug Fixes