Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ legacy-digests = ["sha1", "md5"]
clap = { version = "4.5.0", features = ["derive"] }
serde = { version = "1", features = ["derive"] }
serde_yml = "0.0.12"
minijinja = { version = "2.11.0", features = ["loader"] }
minijinja = { version = "2.12.0", features = ["loader"] }
cap-std = { version = "3.4.4", features = ["fs_utf8"] }
camino = "1.2.0"
semver = { version = "1", features = ["serde"] }
Expand Down
20 changes: 20 additions & 0 deletions docs/netsuke-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,18 @@ unnecessary project risk.
`serde_yml` is mature, widely adopted, and battle-tested, making it the prudent
choice for production-quality software.

**Maintenance risk.** `serde_yml` is archived upstream and carries unsoundness
advisories. Netsuke relies on it today, but we will investigate a maintained
successor such as `serde_yaml_ng`. A follow-up ADR will outline the migration
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (review_instructions): The phrase "we will investigate" uses a first person pronoun, which should be avoided per the instructions.

Consider rephrasing to passive voice, e.g., "A maintained successor such as serde_yaml_ng will be investigated."

Review instructions:

Path patterns: **/*.md

Instructions:
Avoid 2nd person or 1st person pronouns ("I", "you", "we")

plan and compatibility testing.

Follow-up actions:

- Draft an ADR comparing `serde_yml` with candidate replacements and recording
the migration decision.
- Schedule a migration spike to prototype deserialisation with the preferred
crate and capture compatibility notes.

### 3.2 Core Data Structures (`ast.rs`)

The Rust structs that `serde_yml` will deserialise into form the Abstract
Expand Down Expand Up @@ -539,6 +551,7 @@ use netsuke::ast::*;
let ast = NetsukeManifest {
netsuke_version: Version::parse("1.0.0").unwrap(),
vars: HashMap::new(),
macros: vec![],
rules: vec![],
actions: vec![],
targets: vec![Target {
Expand Down Expand Up @@ -720,6 +733,13 @@ If a macro name matches a built-in function or filter, the macro overrides the
built-in definition. This mirrors Jinja's behaviour and follows `minijinja`
semantics where later definitions shadow earlier ones.

The manifest loader compiles each macro definition into an internal template
and registers a wrapper function that evaluates the macro on demand. The
wrapper constructs a fresh MiniJinja state for every invocation so macro calls
do not depend on the lifetime of the manifest parsing state. This preserves
MiniJinja's argument handling, including keyword parameters and `caller`
support, while allowing later macros to override earlier ones.

### 4.4 Essential Custom Functions

To transform `minijinja` from a general-purpose templating engine into a
Expand Down
9 changes: 8 additions & 1 deletion docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ configurations with variables, control flow, and custom functions.
- [x] Implement the critical glob(pattern) custom function to perform file
path globbing, with results sorted lexicographically.

- [ ] Support user-defined Jinja macros declared in a top-level macros list,
- [x] Support user-defined Jinja macros declared in a top-level macros list,
registering them with the environment before rendering.

- **Success Criterion:**
Expand All @@ -117,6 +117,13 @@ configurations with variables, control flow, and custom functions.
custom macros, and the `glob()` function to discover and operate on source
files.

- [ ] **YAML Parser Migration:**

- [ ] Draft an ADR evaluating maintained replacements for `serde_yml`
(for example `serde_yaml_ng`) and record the migration decision.
- [ ] Run a migration spike with the preferred crate, exercising the manifest
fixtures to capture compatibility notes and required mitigations.

## Phase 3: The "Friendly" Polish 🛡️

Objective: To implement the advanced features that deliver a superior, secure,
Expand Down
13 changes: 13 additions & 0 deletions src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ where
/// assert_eq!(manifest.targets.len(), 1);
/// # Ok(()) }
/// ```
#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
pub struct MacroDefinition {
/// Full macro signature as accepted by `MiniJinja`.
pub signature: String,
/// Body of the macro written using YAML block style.
pub body: String,
}

#[derive(Debug, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
pub struct NetsukeManifest {
Expand All @@ -66,6 +75,10 @@ pub struct NetsukeManifest {
#[serde(default)]
pub vars: Vars,

/// Optional list of user-defined Jinja macros registered before rendering.
#[serde(default)]
pub macros: Vec<MacroDefinition>,

/// Named rule templates that can be referenced by targets.
#[serde(default)]
pub rules: Vec<Rule>,
Expand Down
Loading
Loading