feat(cd): implement cd navigation for absolute paths#11
Conversation
📝 WalkthroughWalkthroughAdds support for the "cd" builtin command in src/main.rs. The implementation handles absolute paths by validating they exist and are directories before changing the process current directory using Changes
Sequence DiagramsequenceDiagram
actor User
participant Shell as src/main.rs
participant FileSystem as Filesystem
User->>Shell: Enter cd [path]
Shell->>Shell: Parse arguments
alt Has arguments
Shell->>Shell: Check if absolute path
alt Absolute path (starts with /)
Shell->>FileSystem: Verify path exists & is directory
alt Path exists & is directory
Shell->>Shell: env::set_current_dir(path)
Shell->>User: ✓ Directory changed
else Path invalid or not directory
Shell->>User: ✗ cd: <path>: No such file or directory
end
else Non-absolute path
Shell->>User: ✗ cd: <path>: No such file or directory
end
else No arguments
Shell->>User: (No behavior change)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.rs (1)
39-39: Add "cd" to the builtin list.The builtin list is missing "cd", which means
type cdwill incorrectly report that cd is not a builtin even though it's now implemented as one at lines 79-101.📝 Proposed fix
- let builtin = ["exit", "echo", "type", "pwd"]; + let builtin = ["exit", "echo", "type", "pwd", "cd"];
🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 86-95: The cd handling branch around query.starts_with("/")
should: check args/full_path via Path::new and if !path.exists() or
!path.is_dir() print a clear error (fix the "or or" typo to "or") indicating
whether the target does not exist or is not a directory; when calling
env::set_current_dir(path) capture the Err as e and print a contextual error
message that includes e (e.g., permission denied or underlying OS error) rather
than always saying "No such file or directory"; update the println! calls
accordingly and include the path.display() in each message for clarity.
- Around line 96-100: The code constructs full_path by joining multiple args
(args.join(" ")) and prints "cd: ... No such file or directory" for the
relative-path branch even though the path isn't checked; update the
relative-path handling in main.rs (the branch that reads parts, args, and
full_path) to not assert non-existence—replace that error text with a clear
message like "cd: relative paths not yet supported" and, to follow standard
shell behavior, change both branches that use args.join(" ") to instead accept
exactly one path argument (use args.get(0) or args.first()), returning a concise
error such as "cd: too many arguments" or "cd: missing operand" when applicable.
🧹 Nitpick comments (1)
src/main.rs (1)
80-82: Consider implementing standard home directory behavior.When
cdis invoked without arguments, standard Unix shells change to the user's home directory (typically obtained from theHOMEenvironment variable). Currently, the code just continues without any action. While this doesn't cause errors, implementing standard behavior would improve user experience.🏠 Proposed enhancement
if parts.len() < 2 { - continue; + match env::var("HOME") { + Ok(home) => { + if let Err(e) = env::set_current_dir(home) { + eprintln!("cd: {}", e); + } + } + Err(_) => eprintln!("cd: HOME not set"), + } + continue; }
1 participant
summary
notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.