Clean ./ noise from upload_dir paths in logs

[cloud9c]

Summary

config_dir.join(&wavedash_config.upload_dir) preserves both segments literally, so when a user's `wavedash.toml` has `upload_dir = "./..."` the resolved path rendered as `././...` in every error and log message. Noticed while QAing #28:

```
Error: Multiple HTML files found in upload_dir (././build-rename-test):
```

Add a small `clean_path` helper that collapses `CurDir` (`.`) and resolves `ParentDir` (`..`) without touching the filesystem or pulling in a new dep. Applied at the single site where `upload_dir` is resolved, so everything downstream (error messages, `ServeDir` logs) benefits.

Before / after

`upload_dir = "./nonexistent"`:

• Before: `Error: Upload directory does not exist or is not a directory: ./nonexistent` (or `././nonexistent` depending on CWD)
• After: `Error: Upload directory does not exist or is not a directory: nonexistent`

Test plan

• `cargo build --release` clean
• `upload_dir = "./nonexistent"` → error shows `nonexistent` (no `./`)
• An absolute upload_dir still renders unchanged
• Complements Error when upload_dir contains multiple HTML files and no index.html #28 — rebase/merge order isn't critical; either can land first

🤖 Generated with Claude Code

[claude]

Claude finished @cloud9c's task in 1m 31s —— View job

---

Code Review

• Gather context and read changed files
• Review the implementation
• Post review feedback

---

The fix is correct and well-scoped for the dev command. The clean_path implementation is sound — skipping CurDir, resolving ParentDir only against Normal segments, and guarding against an empty result. No new dependencies, no filesystem I/O. Good.

A few things worth addressing:

---

builds.rs has the same bug (not fixed here)

src/builds.rs:141 has the identical pattern:

let upload_dir = config_dir.join(&wavedash_config.upload_dir);

The same ././... noise will appear in the upload command's error messages:

• Source directory does not exist: ././nonexistent (line 145)
• No files found in ././build (line 157)

clean_path lives in dev/mod.rs and is private, so builds.rs can't use it today. Two options:

1. Move clean_path to a shared module (e.g. src/path_utils.rs or inline in src/lib.rs) and apply it in builds.rs too.
2. File a follow-up issue and note it in the PR description.

Fix this →

---

Edge case: absolute path + excess .. produces /../ instead of /

For an absolute config_dir (e.g. /home/user/project) with upload_dir = "../../..", clean_path produces /../ rather than /:

RootDir  →  push "/"
Normal   →  push "user/project"
ParentDir × 3  →  pop "project", pop "user", then hit RootDir (not Normal) → push ".."
Result: "/../"

This is unlikely in practice and the downstream upload_dir.exists() check immediately rejects it, so it's not a blocker. But worth knowing if the helper gets promoted to a shared util.

---

No unit tests for clean_path

Given the branchy logic (CurDir, ParentDir above root, empty-result guard), a small #[cfg(test)] block would protect against regressions. Suggested cases:

assert_eq!(clean_path(Path::new(".")),              PathBuf::from("."));
assert_eq!(clean_path(Path::new("./build")),        PathBuf::from("build"));
assert_eq!(clean_path(Path::new("././build")),      PathBuf::from("build"));
assert_eq!(clean_path(Path::new("a/b/../c")),       PathBuf::from("a/c"));
assert_eq!(clean_path(Path::new("../foo")),         PathBuf::from("../foo"));
assert_eq!(clean_path(Path::new("/abs/./path")),    PathBuf::from("/abs/path"));

---

Minor: config_parent_dir still returns "." for relative config paths

config_parent_dir("./wavedash.toml") returns "." (not env::current_dir()), since "." is non-empty. That's why clean_path ends up outputting nonexistent rather than /full/path/nonexistent for the default config. This is pre-existing behavior — clean_path doesn't change it — but it means error messages remain relative when using the default config. Not a problem introduced here; just noting the interaction.

---

Summary: The core fix is correct and the approach is idiomatic. The main gap is that builds.rs has the same bug and the helper isn't shared yet. Tests would also strengthen this. The two critical items (builds.rs gap, tests) are worth resolving before merging, or explicitly scoping out with a follow-up issue.

[claude]

Claude finished @cloud9c's task in 1m 14s —— View job

---

Code Review

• Gather context and read changed files
• Review the implementation
• Post review feedback

---

This revision addresses all three gaps from the previous review. Here's the assessment:

What's addressed

Previous concern	Status
builds.rs had the same bug	✅ Fixed — crate::util::clean_path now applied at builds.rs:141
clean_path was private to dev/mod.rs	✅ Moved to shared src/util.rs
/../ edge case with absolute path + excess ..	✅ Sidestepped — new impl doesn't resolve .. at all
No unit tests	❌ Still missing

---

One minor style inconsistency

builds.rs:141 uses a fully-qualified call path:

let upload_dir = crate::util::clean_path(&config_dir.join(&wavedash_config.upload_dir));

while dev/mod.rs imports it:

use crate::util::clean_path;
// ...
let upload_dir = clean_path(&config_dir.join(&wavedash_config.upload_dir));

Should use the use import in builds.rs for consistency. Fix this →

---

Tests are still missing

clean_path has branchy logic (CurDir filter, empty-result guard). Suggested #[cfg(test)] block for src/util.rs:

#[cfg(test)]
mod tests {
    use super::*;
    use std::path::Path;

    #[test]
    fn test_clean_path() {
        assert_eq!(clean_path(Path::new(".")),           PathBuf::from("."));
        assert_eq!(clean_path(Path::new("./build")),     PathBuf::from("build"));
        assert_eq!(clean_path(Path::new("././build")),   PathBuf::from("build"));
        assert_eq!(clean_path(Path::new("a/b")),         PathBuf::from("a/b"));
        assert_eq!(clean_path(Path::new("../foo")),      PathBuf::from("../foo")); // .. passed through
        assert_eq!(clean_path(Path::new("/abs/./path")), PathBuf::from("/abs/path"));
    }
}

Fix this →

---

Summary: The core fix is solid and the builds.rs gap from the previous review is closed. The simplification (dropping .. resolution) is the right call — it avoids the /../ edge case and the doc comment correctly explains the rationale. Only two remaining items: import style inconsistency in builds.rs (trivial) and missing tests (worth adding before merge). Everything else looks good.