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
9 changes: 9 additions & 0 deletions crates/but/src/args/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,18 @@ pub enum Subcommands {
/// ```text
/// but config target origin/main
/// ```
///
/// Set a target branch and push branches to a fork:
///
/// ```text
/// but config target upstream/main --push-remote origin
/// ```
Target {
/// New target branch to set (e.g., "origin/main")
branch: Option<String>,
/// Remote to push branches to (e.g., "origin" for a fork).
#[clap(long, value_name = "REMOTE", requires = "branch")]
push_remote: Option<String>,
},

/// View or set metrics collection.
Expand Down
44 changes: 44 additions & 0 deletions crates/but/src/args/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,50 @@ fn output_format_parses_agent() {
assert!(matches!(args.format.format, OutputFormat::Agent));
}

mod config_target {
use clap::Parser;

use crate::args::{
Args, Subcommands,
config::{Platform as ConfigPlatform, Subcommands as ConfigCmd},
};

#[test]
fn parses_push_remote_for_fork() {
let args = Args::try_parse_from([
"but",
"config",
"target",
"upstream/main",
"--push-remote",
"origin",
])
.expect("parse args");

match args.cmd.expect("subcommand") {
Subcommands::Config(ConfigPlatform {
cmd:
Some(ConfigCmd::Target {
branch,
push_remote,
}),
}) => {
assert_eq!(branch.as_deref(), Some("upstream/main"));
assert_eq!(push_remote.as_deref(), Some("origin"));
}
_ => panic!("unexpected command shape"),
}
}

#[test]
fn push_remote_requires_target_branch() {
let err = Args::try_parse_from(["but", "config", "target", "--push-remote", "origin"])
.expect_err("push remote requires a target branch");

assert_eq!(err.kind(), clap::error::ErrorKind::MissingRequiredArgument);
}
}

#[test]
fn output_format_agent_is_text_without_human_ui() {
let format = OutputFormat::Agent;
Expand Down
27 changes: 20 additions & 7 deletions crates/but/src/command/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ pub async fn exec(
) -> Result<()> {
match cmd {
Some(Subcommands::User { cmd }) => user_config(ctx, out, cmd).await,
Some(Subcommands::Target { branch }) => target_config(ctx, out, branch).await,
Some(Subcommands::Target {
branch,
push_remote,
}) => target_config(ctx, out, branch, push_remote).await,
Some(Subcommands::Forge { cmd }) => forge_config(out, cmd).await,
Some(Subcommands::Metrics { status }) => metrics_config(out, status).await,
Some(Subcommands::Ai { local, global, cmd }) => {
Expand Down Expand Up @@ -1764,6 +1767,7 @@ async fn target_config(
ctx: &mut Context,
out: &mut OutputChannel,
branch: Option<String>,
push_remote: Option<String>,
) -> Result<()> {
let t = theme::get();
match branch {
Expand Down Expand Up @@ -1866,17 +1870,26 @@ async fn target_config(
)?;
}

// from the new_branch string, we need to parse out the remote name and branch name
if let Some(push_remote) = push_remote.as_deref() {
ctx.repo
.get()?
.find_remote(push_remote)
.with_context(|| format!("Failed to find push remote '{push_remote}'"))?;
Comment thread
estib-vega marked this conversation as resolved.
}

let target_ref: gix::refs::FullName = format!("refs/remotes/{new_branch}")
.try_into()
.context("Invalid target branch name")?;
Comment on lines +1880 to +1882
drop((guard, ws));
cfg_if! {
if #[cfg(feature = "legacy")] {
drop((guard, ws));
but_api::legacy::virtual_branches::set_base_branch(
but_api::workspace::set_target_ref_and_init_project(
ctx,
new_branch.clone(),
None,
target_ref.as_ref(),
push_remote,
)?;
Comment thread
estib-vega marked this conversation as resolved.
Comment on lines +1886 to 1890

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.

P2 Badge Recompute the base when changing the target

When a repo already has gitbutler.project.targetCommitId and the user changes to a target whose merge-base differs, this now calls set_target_ref_and_init_project(), which preserves the existing target commit id instead of recomputing it. That leaves targetRef pointing at the new branch while base_sha/targetCommitId still comes from the old target, so target/status/integration calculations using ProjectMeta::target_commit_id operate against the wrong base; the previous set_base_branch path recomputed the merge-base for the requested branch.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems to be encoded in a test. This should be fixed in another PR

} else {
anyhow::bail!("Cannot yet set the base-branch without legacy functions - needs port")
anyhow::bail!("Cannot yet set the target branch without legacy functions")
}
};
}
Expand Down
22 changes: 22 additions & 0 deletions crates/but/tests/but/command/config.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
use crate::utils::{CommandExt as _, Sandbox};
use snapbox::str;

#[cfg(feature = "legacy")]
#[test]
fn target_configures_distinct_push_remote_for_fork() {
let env = Sandbox::open_with_default_settings("repo-with-remote-and-head");
env.but("setup").assert().success();
env.invoke_git("remote add upstream .");
env.invoke_git("update-ref refs/remotes/upstream/main refs/remotes/origin/main");

env.but("config target upstream/main --push-remote origin")
.assert()
.success();

assert_eq!(
env.invoke_git("config --local --get gitbutler.project.targetRef"),
"refs/remotes/upstream/main"
);
assert_eq!(
env.invoke_git("config --local --get gitbutler.project.pushRemote"),
"origin"
);
}

#[test]
fn ai_openai_defaults_to_global_config() {
let env = Sandbox::empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ pub fn writable_context(script_name: &str, repo_name: &str) -> Result<(Context,
let (tmp, _) = gix_testtools::scripted_fixture_writable_with_args_with_post(
script_name.clone(),
None::<String>,
if script_name == "reorder.sh" || script_name == "workspace-commit.sh" {
if script_name == "reorder.sh"
|| script_name == "workspace-commit.sh"
|| script_name == "for-listing.sh"
{
gix_testtools::Creation::Execute
} else {
gix_testtools::Creation::CopyFromReadOnly
Expand Down
2 changes: 1 addition & 1 deletion e2e/playwright/tests/singleBranch/commitActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ function git(pathToRepo: string, args: string[]): string {
function localBranches(pathToRepo: string): string[] {
return git(pathToRepo, ["for-each-ref", "--format=%(refname:short)", "refs/heads"])
.split("\n")
.filter(Boolean);
.filter((branch) => branch && !branch.startsWith("gitbutler/"));
}

async function createGeneratedBranch(
Expand Down
Loading