Skip to content

fix: default new recipe path to current folder#341

Merged
dubadub merged 1 commit into
mainfrom
fix/new-recipe-default-folder
May 15, 2026
Merged

fix: default new recipe path to current folder#341
dubadub merged 1 commit into
mainfrom
fix/new-recipe-default-folder

Conversation

@dubadub
Copy link
Copy Markdown
Member

@dubadub dubadub commented May 15, 2026

Summary

  • Pre-populates the new recipe filename input with the current directory path when "+ New Recipe" is clicked from inside a folder
  • Root listing keeps the existing plain /new link

Fixes #332

Test plan

  • cargo fmt, cargo clippy, cargo test pass
  • From /, the button links to /new (no prefill)
  • From /directory/Breakfast, the button links to /new?filename=Breakfast%2F and the form input renders with value="Breakfast/"
  • Submitting the form with the prefilled value plus a recipe name creates the file in the expected folder

Clicking "+ New Recipe" while browsing a directory now pre-populates
the filename input with that directory path so users can immediately
type the recipe name instead of retyping the folder.
@claude
Copy link
Copy Markdown

claude Bot commented May 15, 2026

Code Review — PR #341: fix: default new recipe path to current folder

Overview: A small, focused fix that pre-populates the "+ New Recipe" link with the current directory path when browsing a folder. Three files changed, 13 net lines.


Verdict: Approve with minor notes


Code Quality

The implementation is clean and idiomatic:

  • urlencoding::encode(p) correctly handles any special characters in the directory path (spaces, non-ASCII, etc.) before embedding it in the query string.
  • The hardcoded %2F suffix to append the directory separator is correct — it pre-fills the form input as Breakfast/, signaling the recipe name will be appended to that prefix.
  • The None branch correctly falls back to the root /new link with no query parameter, preserving existing root-level behavior exactly.

Minor nit: The format string "{}/new?filename={}%2F" mixes a hardcoded percent-encoded character (%2F) with a Rust format placeholder. This is correct but slightly surprising to read. A small inline comment like // trailing %2F marks this as a directory prefix would help future readers understand why it's there rather than using a format placeholder.

Template Safety

Askama HTML-escapes {{ new_recipe_url }} by default. Since urlencoding::encode converts any & in the path to %26, there is no bare & left in the URL string for Askama to escape — so the generated href attribute is well-formed. No XSS risk.

Potential Edge Case

If path is somehow Some("") (empty string), the URL becomes /new?filename=%2F, pre-filling the input with / alone. This is a degenerate case that probably cannot be reached through normal routing, but worth noting.

Test Plan

The PR's manual test plan covers the important cases (root vs. subdirectory, form submission creating the file in the correct folder). Given the project has no automated test suite (noted in CLAUDE.md), this is the appropriate level of verification.


Summary: Well-scoped fix with correct URL encoding, no security concerns, and minimal blast radius. The nits above are truly minor. Ready to merge.

@claude
Copy link
Copy Markdown

claude Bot commented May 15, 2026

Code Review — PR #341

Overview

Pre-populates the new-recipe filename input with the current directory when the "+ New Recipe" button is clicked from inside a folder. Root listing keeps the existing plain /new link.

Code Quality

  • Clean, minimal change: logic lives in the Rust handler (not the template), which is the right place.
  • urlencoding is already a workspace dependency and used elsewhere in ui.rs, so no new dependency is introduced.
  • Template change is a one-liner replacing a hardcoded prefix reference.

Potential Issues

  • Hardcoded %2F suffix: The URL is built as format!("{}/new?filename={}%2F", …, urlencoding::encode(p)). The trailing %2F is correct but slightly fragile compared to encoding the full intended value. Consider:
    format!("{}/new?filename={}", state.url_prefix, urlencoding::encode(&format!("{}/", p)))
    This is cosmetic — functionally both are equivalent.
  • Security: The pre-filled filename value flows through the existing POST handler's sanitization (character allowlist + path traversal check), so there is no new attack surface here. Good.
  • Empty path edge case: The None branch correctly produces /new with no query string, matching previous behavior.

Verdict

Looks good. Correct, safe, and minimal. The hardcoded %2F nit is cosmetic — fine to ship as-is.

@dubadub dubadub merged commit f8bde38 into main May 15, 2026
6 checks passed
@dubadub dubadub deleted the fix/new-recipe-default-folder branch May 15, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Server] Click "+ New Recipe" on cooklang web server interface does not default the path to the folder that the user is currently in.

1 participant