From bb4ab5035d1639be74a2e91ba6a2e049d069114d Mon Sep 17 00:00:00 2001 From: hippi345 <13539685+hippi345@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:56:59 +0000 Subject: [PATCH 1/2] fix(notebooks): set name when creating from Markdown Markdown create posted the file body as-is, so the first heading became cell content and data.attributes.name stayed empty. Add --name for Markdown create. When omitted, use the first ATX heading (any level). If there is no heading, leave the name empty as before. The Markdown API stores the name in YAML frontmatter title. --- docs/COMMANDS.md | 7 +- src/commands/notebooks.rs | 295 +++++++++++++++++++++++++++++++++++++- src/main.rs | 22 ++- 3 files changed, 314 insertions(+), 10 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index e0ccde91..d0dd44ff 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -338,7 +338,11 @@ notebook as a Markdown document instead of a JSON cells array. These call `/api/v2`; the path and response contract may still change. - `get --markdown` — print the notebook as Markdown (YAML frontmatter + body) -- `create --markdown --file doc.md` — create from a Markdown file; prints the new id +- `create --markdown --file doc.md` — create from a Markdown file; prints the new id. + Optional `--name` sets the notebook name (YAML frontmatter `title`, the Markdown + form of `data.attributes.name`). Without `--name`, the first ATX heading + (`#`–`######`, any level; setext headings are ignored) is used. If there is no + heading either, the name is left empty. - `update --markdown --file doc.md` — replace the whole document - `edit --markdown --file fragment.md` — append the fragment server-side @@ -346,6 +350,7 @@ notebook as a Markdown document instead of a JSON cells array. These call ```bash ID=$(pup notebooks create --markdown --file doc.md) +# or: pup notebooks create --markdown --file doc.md --name "Weekly review" pup notebooks get "$ID" --markdown ``` diff --git a/src/commands/notebooks.rs b/src/commands/notebooks.rs index 21a88887..95eba35c 100644 --- a/src/commands/notebooks.rs +++ b/src/commands/notebooks.rs @@ -110,6 +110,116 @@ fn decode_markdown_response(resp: raw_client::HttpResponse) -> Result { .map_err(|e| anyhow::anyhow!("notebook Markdown response was not valid UTF-8: {e}")) } +/// First ATX heading (`#`–`######`) outside fenced code, used as the default +/// notebook name for Markdown create. Setext headings are ignored. +fn first_atx_heading(content: &str) -> Option { + let mut in_fence = false; + for line in content.lines() { + let trimmed = line.trim_start(); + if trimmed.starts_with("```") || trimmed.starts_with("~~~") { + in_fence = !in_fence; + continue; + } + if in_fence { + continue; + } + let hashes = trimmed.chars().take_while(|c| *c == '#').count(); + if !(1..=6).contains(&hashes) { + continue; + } + let rest = &trimmed[hashes..]; + let rest = rest.strip_prefix(' ').or_else(|| rest.strip_prefix('\t')); + let Some(rest) = rest else { + continue; + }; + let text = rest.trim().trim_end_matches('#').trim(); + if !text.is_empty() { + return Some(text.to_string()); + } + } + None +} + +fn yaml_double_quoted(value: &str) -> String { + let mut out = String::from("\""); + for c in value.chars() { + match c { + '\\' => out.push_str("\\\\"), + '"' => out.push_str("\\\""), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + _ => out.push(c), + } + } + out.push('"'); + out +} + +fn split_yaml_frontmatter(content: &str) -> Option<(String, String)> { + let content = content.trim_start_matches('\u{feff}'); + let rest = content + .strip_prefix("---\n") + .or_else(|| content.strip_prefix("---\r\n"))?; + let mut frontmatter = String::new(); + let mut lines = rest.split_inclusive('\n'); + for line in lines.by_ref() { + if line.trim_end_matches(['\r', '\n']) == "---" { + return Some((frontmatter, lines.collect())); + } + frontmatter.push_str(line); + } + None +} + +fn upsert_yaml_title(frontmatter: &str, quoted_title: &str) -> String { + let mut replaced = false; + let mut out = String::new(); + for line in frontmatter.split_inclusive('\n') { + let trimmed = line.trim_start(); + if !replaced && trimmed.starts_with("title:") { + let newline = if line.ends_with('\n') { "\n" } else { "" }; + out.push_str("title: "); + out.push_str(quoted_title); + out.push_str(newline); + replaced = true; + } else { + out.push_str(line); + } + } + if !replaced { + if !out.is_empty() && !out.ends_with('\n') { + out.push('\n'); + } + out.push_str("title: "); + out.push_str(quoted_title); + out.push('\n'); + } + out +} + +/// YAML `title` is how `/api/unstable/notebooks` round-trips the notebook name +/// (see Markdown GET). `--name` wins; otherwise the first ATX heading is used. +/// No heading leaves the document unchanged so the API still stores `name: ""`. +fn markdown_create_document(content: &str, name: Option<&str>) -> String { + let title = name + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) + .or_else(|| first_atx_heading(content)); + let Some(title) = title else { + return content.to_string(); + }; + let quoted = yaml_double_quoted(&title); + if let Some((frontmatter, body)) = split_yaml_frontmatter(content) { + format!( + "---\n{}---\n{body}", + upsert_yaml_title(&frontmatter, "ed) + ) + } else { + format!("---\ntitle: {quoted}\n---\n\n{content}") + } +} + fn read_markdown_file(file: &str) -> Result { // The endpoints take a JSON file verbatim as prose, so catch it here. let content = @@ -274,9 +384,12 @@ pub async fn delete(cfg: &Config, notebook_id: i64) -> Result<()> { Ok(()) } -pub async fn create(cfg: &Config, file: &str, markdown: bool) -> Result<()> { +pub async fn create(cfg: &Config, file: &str, markdown: bool, name: Option<&str>) -> Result<()> { + if name.is_some() && !markdown { + anyhow::bail!("--name can only be used with --markdown"); + } if markdown { - let content = read_markdown_file(file)?; + let content = markdown_create_document(&read_markdown_file(file)?, name); let resp = markdown_request( cfg, "POST", @@ -513,7 +626,7 @@ mod tests { .mock("POST", "/api/unstable/notebooks") .match_header("content-type", "text/markdown") .match_header("accept", "application/vnd.api+json") - .match_body("## created\n") + .match_body("---\ntitle: \"created\"\n---\n\n## created\n") .with_status(201) .with_header("content-type", "application/vnd.api+json") .with_body(r###"{"data":{"id":"987654","attributes":{"markdown":"## created\n"}}}"###) @@ -521,7 +634,7 @@ mod tests { .await; let path = write_temp_json("pup_nb_create_id.md", "## created\n"); - let result = super::create(&cfg, path.to_str().unwrap(), true).await; + let result = super::create(&cfg, path.to_str().unwrap(), true, None).await; let _ = std::fs::remove_file(path); assert!(result.is_ok(), "create failed: {:?}", result.err()); @@ -690,7 +803,7 @@ mod tests { .mock("POST", "/api/unstable/notebooks") .match_header("content-type", "text/markdown") .match_header("accept", "application/vnd.api+json") - .match_body("## new notebook\n") + .match_body("---\ntitle: \"new notebook\"\n---\n\n## new notebook\n") .with_status(201) .with_header("content-type", "application/vnd.api+json") .with_body(r###"{"data":{"id":"1","attributes":{"markdown":"## new notebook\n"}}}"###) @@ -698,7 +811,122 @@ mod tests { .await; let path = write_temp_json("pup_nb_create_md.md", "## new notebook\n"); - let result = super::create(&cfg, path.to_str().unwrap(), true).await; + let result = super::create(&cfg, path.to_str().unwrap(), true, None).await; + let _ = std::fs::remove_file(path); + + assert!(result.is_ok(), "create failed: {:?}", result.err()); + mock.assert_async().await; + cleanup_env(); + } + + #[test] + fn test_markdown_create_name_from_explicit_flag() { + assert_eq!( + super::markdown_create_document("# Heading in file\n\nHello\n", Some("From flag")), + "---\ntitle: \"From flag\"\n---\n\n# Heading in file\n\nHello\n" + ); + } + + #[test] + fn test_markdown_create_name_from_first_atx_heading() { + assert_eq!( + super::markdown_create_document("# Test notebook\n\nHello world!\n", None), + "---\ntitle: \"Test notebook\"\n---\n\n# Test notebook\n\nHello world!\n" + ); + assert_eq!( + super::first_atx_heading("intro\n\n## Secondary title\n"), + Some("Secondary title".into()), + "any ATX level, not only #" + ); + assert_eq!( + super::first_atx_heading("```\n# fenced\n```\n# real\n"), + Some("real".into()) + ); + } + + #[test] + fn test_markdown_create_name_without_heading_leaves_document_unchanged() { + let body = "Hello world!\nNo heading here.\n"; + assert_eq!(super::markdown_create_document(body, None), body); + assert_eq!(super::first_atx_heading(body), None); + assert_eq!( + super::markdown_create_document(body, Some(" ")), + body, + "blank --name is treated as unset" + ); + } + + #[tokio::test] + async fn test_notebooks_create_markdown_uses_name_flag() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + let mock = server + .mock("POST", "/api/unstable/notebooks") + .match_header("content-type", "text/markdown") + .match_body("---\ntitle: \"From flag\"\n---\n\n# Test notebook\n\nHello world!\n") + .with_status(201) + .with_header("content-type", "application/vnd.api+json") + .with_body(r###"{"data":{"id":"1"}}"###) + .create_async() + .await; + + let path = write_temp_json( + "pup_nb_create_name_flag.md", + "# Test notebook\n\nHello world!\n", + ); + let result = super::create(&cfg, path.to_str().unwrap(), true, Some("From flag")).await; + let _ = std::fs::remove_file(path); + + assert!(result.is_ok(), "create failed: {:?}", result.err()); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_notebooks_create_markdown_defaults_name_from_first_heading() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + let mock = server + .mock("POST", "/api/unstable/notebooks") + .match_header("content-type", "text/markdown") + .match_body("---\ntitle: \"Test notebook\"\n---\n\n# Test notebook\n\nHello world!\n") + .with_status(201) + .with_header("content-type", "application/vnd.api+json") + .with_body(r###"{"data":{"id":"1"}}"###) + .create_async() + .await; + + let path = write_temp_json( + "pup_nb_create_name_heading.md", + "# Test notebook\n\nHello world!\n", + ); + let result = super::create(&cfg, path.to_str().unwrap(), true, None).await; + let _ = std::fs::remove_file(path); + + assert!(result.is_ok(), "create failed: {:?}", result.err()); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_notebooks_create_markdown_without_heading_posts_body_unchanged() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + let mock = server + .mock("POST", "/api/unstable/notebooks") + .match_header("content-type", "text/markdown") + .match_body("Hello world!\n") + .with_status(201) + .with_header("content-type", "application/vnd.api+json") + .with_body(r###"{"data":{"id":"1"}}"###) + .create_async() + .await; + + let path = write_temp_json("pup_nb_create_name_none.md", "Hello world!\n"); + let result = super::create(&cfg, path.to_str().unwrap(), true, None).await; let _ = std::fs::remove_file(path); assert!(result.is_ok(), "create failed: {:?}", result.err()); @@ -706,6 +934,59 @@ mod tests { cleanup_env(); } + #[tokio::test] + async fn test_notebooks_create_rejects_name_without_markdown() { + let _lock = lock_env().await; + let cfg = test_config("http://unused.local"); + let err = super::create(&cfg, "/nonexistent.json", false, Some("x")) + .await + .unwrap_err() + .to_string(); + assert!( + err.contains("--name can only be used with --markdown"), + "{err}" + ); + cleanup_env(); + } + + #[test] + fn test_notebooks_create_name_flag_requires_markdown() { + use clap::Parser; + let result = crate::Cli::try_parse_from([ + "pup", + "notebooks", + "create", + "--file", + "doc.md", + "--name", + "Only with markdown", + ]); + assert!( + result.is_err(), + "--name without --markdown must be rejected" + ); + + let cli = crate::Cli::try_parse_from([ + "pup", + "notebooks", + "create", + "--markdown", + "--file", + "doc.md", + "--name", + "Incident notes", + ]) + .expect("create --markdown --name should parse"); + let crate::Commands::Notebooks { action } = cli.command else { + panic!("expected notebooks"); + }; + let crate::NotebookActions::Create { markdown, name, .. } = action else { + panic!("expected create"); + }; + assert!(markdown); + assert_eq!(name.as_deref(), Some("Incident notes")); + } + #[tokio::test] async fn test_notebooks_update_markdown_patches_document() { let _lock = lock_env().await; @@ -768,7 +1049,7 @@ mod tests { .await; let path = write_temp_json("pup_nb_wrong_flag.json", r#"{"data":{"id":"1"}}"#); - let result = super::create(&cfg, path.to_str().unwrap(), true).await; + let result = super::create(&cfg, path.to_str().unwrap(), true, None).await; let _ = std::fs::remove_file(path); assert!(result.is_err()); diff --git a/src/main.rs b/src/main.rs index 78fe4ad6..277ea777 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2251,6 +2251,10 @@ enum Commands { /// # Create a notebook from file /// pup notebooks create --file notebook.json /// + /// # Create from Markdown (name from --name, else the first ATX heading) + /// pup notebooks create --markdown --file notebook.md + /// pup notebooks create --markdown --file notebook.md --name "Incident notes" + /// /// # Update a notebook /// pup notebooks update 12345 --file updated.json /// @@ -7315,6 +7319,16 @@ enum NotebookActions { Prints the new notebook id; use 'get --markdown' to read the document back" )] markdown: bool, + #[arg( + long, + requires = "markdown", + help = "Notebook name for Markdown create (maps to YAML frontmatter `title`, \ + the Markdown projection of data.attributes.name). Defaults to the text \ + of the first ATX heading (`#`–`######`, any level; setext headings are \ + ignored). If neither --name nor a heading is present, the name is left \ + empty to match previous Markdown create behavior" + )] + name: Option, }, /// Update a notebook (full replace) Update { @@ -16989,9 +17003,13 @@ async fn main_inner() -> anyhow::Result<()> { reject_jq_with_markdown(jq_flag_passed, markdown)?; commands::notebooks::get(&cfg, notebook_id, markdown).await?; } - NotebookActions::Create { file, markdown } => { + NotebookActions::Create { + file, + markdown, + name, + } => { reject_jq_with_markdown(jq_flag_passed, markdown)?; - commands::notebooks::create(&cfg, &file, markdown).await?; + commands::notebooks::create(&cfg, &file, markdown, name.as_deref()).await?; } NotebookActions::Update { notebook_id, From a6aac77aaf3438ea34f931b3d77b895f216eb29e Mon Sep 17 00:00:00 2001 From: hippi345 <13539685+hippi345@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:05:43 +0000 Subject: [PATCH 2/2] fix(notebooks): only upsert top-level YAML title on Markdown create upsert_yaml_title trimmed each frontmatter line before looking for title:, so a nested mapping key or a title: line inside a block scalar was replaced instead of inserting a top-level title. --- src/commands/notebooks.rs | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/src/commands/notebooks.rs b/src/commands/notebooks.rs index 95eba35c..d7a16ca2 100644 --- a/src/commands/notebooks.rs +++ b/src/commands/notebooks.rs @@ -171,12 +171,24 @@ fn split_yaml_frontmatter(content: &str) -> Option<(String, String)> { None } +fn is_top_level_yaml_title_key(line: &str) -> bool { + let line = line.trim_end_matches(['\r', '\n']); + // Nested mappings and block-scalar lines are indented; do not treat those + // `title:` occurrences as the notebook name. + if line.starts_with(' ') || line.starts_with('\t') { + return false; + } + let Some(rest) = line.strip_prefix("title:") else { + return false; + }; + rest.is_empty() || rest.starts_with([' ', '\t']) +} + fn upsert_yaml_title(frontmatter: &str, quoted_title: &str) -> String { let mut replaced = false; let mut out = String::new(); for line in frontmatter.split_inclusive('\n') { - let trimmed = line.trim_start(); - if !replaced && trimmed.starts_with("title:") { + if !replaced && is_top_level_yaml_title_key(line) { let newline = if line.ends_with('\n') { "\n" } else { "" }; out.push_str("title: "); out.push_str(quoted_title); @@ -987,6 +999,27 @@ mod tests { assert_eq!(name.as_deref(), Some("Incident notes")); } + #[test] + fn test_markdown_create_preserves_nested_and_block_scalar_title_keys() { + let nested = "---\nmeta:\n title: nested\n---\n\n# From heading\n"; + assert_eq!( + super::markdown_create_document(nested, Some("From flag")), + "---\nmeta:\n title: nested\ntitle: \"From flag\"\n---\n\n# From heading\n" + ); + + let block = "---\nnote: |\n title: inside block\n---\n\n# From heading\n"; + assert_eq!( + super::markdown_create_document(block, Some("From flag")), + "---\nnote: |\n title: inside block\ntitle: \"From flag\"\n---\n\n# From heading\n" + ); + + let top_level = "---\ntitle: old\nstatus: published\n---\n\nHello\n"; + assert_eq!( + super::markdown_create_document(top_level, Some("From flag")), + "---\ntitle: \"From flag\"\nstatus: published\n---\n\nHello\n" + ); + } + #[tokio::test] async fn test_notebooks_update_markdown_patches_document() { let _lock = lock_env().await;