From 7811643c5012684c2ef8f8bb899b378a5ba34029 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Tue, 3 Feb 2026 17:59:03 +0100 Subject: [PATCH] Add PR message flags for non-interactive use Update PR message docs in skill references Limit custom PR message to selected branch --- crates/but/skill/SKILL.md | 3 + crates/but/skill/references/reference.md | 7 +- crates/but/src/args/forge.rs | 6 + crates/but/src/command/legacy/forge/review.rs | 119 +++++++++++++++--- crates/but/src/lib.rs | 55 ++++++-- 5 files changed, 157 insertions(+), 33 deletions(-) diff --git a/crates/but/skill/SKILL.md b/crates/but/skill/SKILL.md index cd516c80f83..90b9c821659 100644 --- a/crates/but/skill/SKILL.md +++ b/crates/but/skill/SKILL.md @@ -116,6 +116,9 @@ For detailed command syntax and all available options, see [references/reference - `but pull` - Update with upstream - `but push [branch]` - Push to remote - `but pr new ` - Push and create pull request (auto-pushes, no need to push first) +- `but pr new -m "Title..."` - Inline PR message (first line is title, rest is description) +- `but pr new -F pr_message.txt` - PR message from file (first line is title, rest is description) +- For stacked branches, the custom message (`-m` or `-F`) only applies to the selected branch; dependent branches use defaults ## Key Concepts diff --git a/crates/but/skill/references/reference.md b/crates/but/skill/references/reference.md index c636bcc5f8d..616d8e8322b 100644 --- a/crates/but/skill/references/reference.md +++ b/crates/but/skill/references/reference.md @@ -330,13 +330,18 @@ Create and manage pull requests. ```bash but pr new # Push branch and create PR (recommended) -but pr new -m "Title" -m "Body" # With title and description +but pr new -F pr_message.txt # Use file: first line is title, rest is description +but pr new -m "Title..." # Inline message: first line is title, rest is description but pr # Create PR (prompts for branch) but pr template # Configure PR description template ``` **Key behavior:** `but pr new` automatically pushes the branch to remote before creating the PR. No need to run `but push` first. +In non-interactive environments, use `--message (-m)`, `--file (-F)`, or `--default (-t)` to avoid editor prompts. + +**Note:** For stacked branches, the custom message (`-m` or `-F`) only applies to the selected branch. Dependent branches in the stack will use default messages (commit title/description). + Requires forge integration to be configured via `but config forge auth`. ### `but merge ` diff --git a/crates/but/src/args/forge.rs b/crates/but/src/args/forge.rs index eea2993ea0b..0f5583a21d2 100644 --- a/crates/but/src/args/forge.rs +++ b/crates/but/src/args/forge.rs @@ -13,6 +13,12 @@ pub mod pr { /// The branch to create a PR for. #[clap(value_name = "BRANCH")] branch: Option, + /// PR title and description. The first line is the title, the rest is the description. + #[clap(short = 'm', long = "message", conflicts_with_all = &["file", "default"])] + message: Option, + /// Read PR title and description from file. The first line is the title, the rest is the description. + #[clap(short = 'F', long = "file", value_name = "FILE", conflicts_with_all = &["message", "default"])] + file: Option, /// Force push even if it's not fast-forward (defaults to true). #[clap(long, short = 'f', default_value_t = true)] with_force: bool, diff --git a/crates/but/src/command/legacy/forge/review.rs b/crates/but/src/command/legacy/forge/review.rs index 397bb2a9e4e..f50fd590f74 100644 --- a/crates/but/src/command/legacy/forge/review.rs +++ b/crates/but/src/command/legacy/forge/review.rs @@ -63,6 +63,7 @@ pub fn set_review_template( /// Create a new PR for a branch. /// If no branch is specified, prompts the user to select one. /// If there is only one branch without a PR, asks for confirmation. +#[allow(clippy::too_many_arguments)] pub async fn create_pr( ctx: &mut Context, branch: Option, @@ -70,6 +71,7 @@ pub async fn create_pr( with_force: bool, run_hooks: bool, default: bool, + message: Option, out: &mut OutputChannel, ) -> anyhow::Result<()> { let review_map = get_review_map(ctx, Some(but_forge::CacheConfig::CacheOnly))?; @@ -117,6 +119,7 @@ pub async fn create_pr( with_force, run_hooks, default, + message.as_ref(), out, maybe_branch_names, ) @@ -170,6 +173,7 @@ pub async fn handle_multiple_branches_in_workspace( with_force: bool, run_hooks: bool, default_message: bool, + message: Option<&PrMessage>, out: &mut OutputChannel, selected_branches: Option>, ) -> anyhow::Result<()> { @@ -209,6 +213,7 @@ pub async fn handle_multiple_branches_in_workspace( with_force, run_hooks, default_message, + message, out, ) .await?; @@ -327,6 +332,7 @@ async fn publish_reviews_for_branch_and_dependents( with_force: bool, run_hooks: bool, default_message: bool, + message: Option<&PrMessage>, out: &mut OutputChannel, ) -> Result { let base_branch = gitbutler_branch_actions::base::get_base_branch_data(ctx)?; @@ -380,6 +386,7 @@ async fn publish_reviews_for_branch_and_dependents( )?; } + let message_for_head = if head.name == branch_name { message } else { None }; let published_review = publish_review_for_branch( ctx, stack_entry.id, @@ -387,6 +394,7 @@ async fn publish_reviews_for_branch_and_dependents( current_target_branch, review_map, default_message, + message_for_head, ) .await?; match published_review { @@ -482,6 +490,31 @@ enum PublishReviewResult { AlreadyExists(Vec), } +#[derive(Clone, Debug)] +pub struct PrMessage { + pub title: String, + pub body: String, +} + +pub fn parse_pr_message(content: &str) -> anyhow::Result { + let mut lines = content.lines(); + let title = lines.next().unwrap_or("").trim().to_string(); + + if title.is_empty() { + anyhow::bail!("Aborting due to empty PR title"); + } + + // Skip any leading blank lines after the title, then collect the rest as description + let body = lines + .skip_while(|l| l.trim().is_empty()) + .collect::>() + .join("\n") + .trim() + .to_string(); + + Ok(PrMessage { title, body }) +} + async fn publish_review_for_branch( ctx: &mut Context, stack_id: Option, @@ -489,6 +522,7 @@ async fn publish_review_for_branch( target_branch: &str, review_map: &std::collections::HashMap>, default_message: bool, + message: Option<&PrMessage>, ) -> anyhow::Result { // Check if a review already exists for the branch. // If it does, skip publishing a new review. @@ -500,7 +534,9 @@ async fn publish_review_for_branch( } let commit = default_commit(ctx, stack_id, branch_name)?; - let (title, body) = if default_message { + let (title, body) = if let Some(message) = message { + (message.title.clone(), message.body.clone()) + } else if default_message { let title = extract_commit_title(commit.as_ref()) .map(|t| t.to_string()) .unwrap_or(branch_name.to_string()); @@ -658,24 +694,8 @@ fn get_pr_title_and_body_from_editor( template.push_str("#\n"); let content = get_text::from_editor_no_comments("pr_message", &template)?.to_string(); - - // Split into title (first line) and body (rest) - let mut lines = content.lines(); - let title = lines.next().unwrap_or("").trim().to_string(); - - if title.is_empty() { - anyhow::bail!("Aborting due to empty PR title"); - } - - // Skip any leading blank lines after the title, then collect the rest as description - let body: String = lines - .skip_while(|l| l.trim().is_empty()) - .collect::>() - .join("\n") - .trim() - .to_string(); - - Ok((title, body)) + let message = parse_pr_message(&content)?; + Ok((message.title, message.body)) } /// Extract the commit description (body) from the commit message, skipping the first line (title). @@ -756,3 +776,64 @@ pub fn get_review_numbers( "".to_string().normal() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_pr_message_title_only() { + let msg = parse_pr_message("My PR Title").unwrap(); + assert_eq!(msg.title, "My PR Title"); + assert_eq!(msg.body, ""); + } + + #[test] + fn parse_pr_message_title_and_body() { + let msg = parse_pr_message("My PR Title\n\nThis is the body.").unwrap(); + assert_eq!(msg.title, "My PR Title"); + assert_eq!(msg.body, "This is the body."); + } + + #[test] + fn parse_pr_message_multiline_body() { + let msg = parse_pr_message("Title\n\nLine 1\nLine 2\nLine 3").unwrap(); + assert_eq!(msg.title, "Title"); + assert_eq!(msg.body, "Line 1\nLine 2\nLine 3"); + } + + #[test] + fn parse_pr_message_skips_blank_lines_between_title_and_body() { + let msg = parse_pr_message("Title\n\n\n\nBody starts here").unwrap(); + assert_eq!(msg.title, "Title"); + assert_eq!(msg.body, "Body starts here"); + } + + #[test] + fn parse_pr_message_trims_whitespace() { + let msg = parse_pr_message(" Title with spaces \n\n Body with spaces ").unwrap(); + assert_eq!(msg.title, "Title with spaces"); + assert_eq!(msg.body, "Body with spaces"); + } + + #[test] + fn parse_pr_message_empty_string_fails() { + let result = parse_pr_message(""); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("empty PR title")); + } + + #[test] + fn parse_pr_message_whitespace_only_fails() { + let result = parse_pr_message(" \n\n "); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("empty PR title")); + } + + #[test] + fn parse_pr_message_blank_first_line_fails() { + let result = parse_pr_message("\nActual title on second line"); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("empty PR title")); + } +} diff --git a/crates/but/src/lib.rs b/crates/but/src/lib.rs index a5080cfbb91..23270f324ed 100644 --- a/crates/but/src/lib.rs +++ b/crates/but/src/lib.rs @@ -733,22 +733,51 @@ async fn match_subcommand( match cmd { Some(forge::pr::Subcommands::New { branch, + message, + file, skip_force_push_protection, with_force, run_hooks, default, - }) => command::legacy::forge::review::create_pr( - &mut ctx, - branch, - skip_force_push_protection, - with_force, - run_hooks, - default, - out, - ) - .await - .context("Failed to create PR for branch.") - .emit_metrics(metrics_ctx), + }) => { + // Read message content from file or inline + let message_content = match &file { + Some(path) => Some( + std::fs::read_to_string(path) + .with_context(|| format!("Failed to read PR message from file: {}", path.display()))?, + ), + None => message.clone(), + }; + // Parse early to fail fast on invalid content + let pr_message = match message_content { + Some(content) => Some(command::legacy::forge::review::parse_pr_message(&content)?), + None => None, + }; + // Check for non-interactive environment + if !out.can_prompt() { + if branch.is_none() { + anyhow::bail!("Non-interactive environment detected. Please specify a branch."); + } + if pr_message.is_none() && !default { + anyhow::bail!( + "Non-interactive environment detected. Provide one of: --message (-m), --file (-F), or --default (-t)." + ); + } + } + command::legacy::forge::review::create_pr( + &mut ctx, + branch, + skip_force_push_protection, + with_force, + run_hooks, + default, + pr_message, + out, + ) + .await + .context("Failed to create PR for branch.") + .emit_metrics(metrics_ctx) + } Some(forge::pr::Subcommands::Template { template_path }) => { command::legacy::forge::review::set_review_template(&mut ctx, template_path, out) .context("Failed to set PR template.") @@ -756,7 +785,7 @@ async fn match_subcommand( } None => { // Default to `pr new` when no subcommand is provided - command::legacy::forge::review::create_pr(&mut ctx, None, false, true, true, false, out) + command::legacy::forge::review::create_pr(&mut ctx, None, false, true, true, false, None, out) .await .context("Failed to create PR for branch.") .emit_metrics(metrics_ctx)