Conversation
📝 WalkthroughWalkthroughSequentializes absolute-path cd validation: first checks existence, then directory-ness, then attempts to set the current directory. Error messages now use the original input path for "No such file or directory" and "Not a directory", and include the OS error when set_current_dir fails. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 91-99: The conditional checks are inverted and use a non-existent
method; change the first check to if !path.exists() to report "No such file or
directory", replace path.dir() with path.is_dir() to test directoryness, and
send all error messages to stderr by using eprintln! instead of println!
(including the env::set_current_dir error message that currently prints
path.display() and the error), leaving the successful
env::set_current_dir(full_path) branch as-is.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main.rs
🧰 Additional context used
🪛 GitHub Actions: Rust
src/main.rs
[error] 93-93: no method named dir found for &std::path::Path in the current scope. There is a method is_dir with a similar purpose.
[error] 93-93: could not compile rshell (bin "rshell") due to 1 previous error
src/main.rs
Outdated
| if path.exists() { | ||
| println!("cd: {}: No such file or directory", full_path); | ||
| } else if !path.dir() { | ||
| println!("cd: {}: Not a directory", full_path); | ||
| } else { | ||
| if let Err(e) = env::set_current_dir(full_path) { | ||
| println!("cd: {} {}", path.display(), e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix inverted logic and compilation error.
The condition logic is backwards and there's a method name error:
-
Critical logic error (line 91): The condition
if path.exists()will print "No such file or directory" when the path EXISTS, which is contradictory. It should beif !path.exists(). -
Critical compilation error (line 93):
path.dir()doesn't exist. The correct method ispath.is_dir()(as shown in the pipeline failure). -
Error output location: Error messages should use
eprintln!instead ofprintln!to write to stderr rather than stdout, following standard Unix conventions.
🔧 Proposed fix
let path = Path::new(&full_path);
- if path.exists() {
- println!("cd: {}: No such file or directory", full_path);
- } else if !path.dir() {
- println!("cd: {}: Not a directory", full_path);
+ if !path.exists() {
+ eprintln!("cd: {}: No such file or directory", full_path);
+ } else if !path.is_dir() {
+ eprintln!("cd: {}: Not a directory", full_path);
} else {
if let Err(e) = env::set_current_dir(full_path) {
- println!("cd: {} {}", path.display(), e);
+ eprintln!("cd: {} {}", path.display(), e);
}
}📝 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.
| if path.exists() { | |
| println!("cd: {}: No such file or directory", full_path); | |
| } else if !path.dir() { | |
| println!("cd: {}: Not a directory", full_path); | |
| } else { | |
| if let Err(e) = env::set_current_dir(full_path) { | |
| println!("cd: {} {}", path.display(), e); | |
| } | |
| } | |
| if !path.exists() { | |
| eprintln!("cd: {}: No such file or directory", full_path); | |
| } else if !path.is_dir() { | |
| eprintln!("cd: {}: Not a directory", full_path); | |
| } else { | |
| if let Err(e) = env::set_current_dir(full_path) { | |
| eprintln!("cd: {} {}", path.display(), e); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Rust
[error] 93-93: no method named dir found for &std::path::Path in the current scope. There is a method is_dir with a similar purpose.
[error] 93-93: could not compile rshell (bin "rshell") due to 1 previous error
🤖 Prompt for AI Agents
In @src/main.rs around lines 91 - 99, The conditional checks are inverted and
use a non-existent method; change the first check to if !path.exists() to report
"No such file or directory", replace path.dir() with path.is_dir() to test
directoryness, and send all error messages to stderr by using eprintln! instead
of println! (including the env::set_current_dir error message that currently
prints path.display() and the error), leaving the successful
env::set_current_dir(full_path) branch as-is.
1 participant
summary
notes
fixes #12
Summary by CodeRabbit
cdcommand with improved distinction between non-existent paths and non-directory paths. Error messages now include the full user input and underlying system errors for better diagnostics and troubleshooting.✏️ Tip: You can customize this high-level summary in your review settings.