Skip to content

Commit 96b1b4c

Browse files
Branch name slash trimming (#65)
* fix: trim trailing slashes for branch parameters Co-authored-by: Safia Abdalla <safia@safia.rocks> * test: remove low-value utils slash tests Co-authored-by: Safia Abdalla <safia@safia.rocks> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 714c07e commit 96b1b4c

7 files changed

Lines changed: 175 additions & 27 deletions

File tree

src/commands/go.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ use crate::commands::shell_init::{
66
};
77
use crate::git::{discover_repo, find_worktree_by_name, list_worktrees, RepoContext};
88
use crate::models::Worktree;
9-
use crate::utils::get_shell_for_platform;
9+
use crate::utils::{get_shell_for_platform, trim_trailing_branch_slashes};
1010

1111
pub fn run(name: Option<&str>, path_only: bool) {
1212
if path_only
13-
&& name.map(|n| n.trim().is_empty()).unwrap_or(true)
13+
&& name
14+
.map(|n| trim_trailing_branch_slashes(n).is_empty())
15+
.unwrap_or(true)
1416
&& !atty::is(atty::Stream::Stdin)
1517
{
1618
eprintln!(
@@ -30,10 +32,11 @@ pub fn run(name: Option<&str>, path_only: bool) {
3032
};
3133

3234
let worktree = if let Some(name) = name {
33-
if name.trim().is_empty() {
35+
let normalized_name = trim_trailing_branch_slashes(name);
36+
if normalized_name.is_empty() {
3437
pick_or_error(&repo)
3538
} else {
36-
match find_worktree_by_name(&repo, name) {
39+
match find_worktree_by_name(&repo, normalized_name) {
3740
Ok(Some(wt)) => wt,
3841
Ok(None) => {
3942
eprintln!(

src/commands/prune.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::git::{
66
DETACHED_HEAD,
77
};
88
use crate::models::Worktree;
9-
use crate::utils::parse_duration;
9+
use crate::utils::{parse_duration, trim_trailing_branch_slashes};
1010

1111
pub fn run(dry_run: bool, force: bool, base: Option<&str>, older_than: Option<&str>) {
1212
if older_than.is_some() && base.is_some() {
@@ -32,7 +32,12 @@ pub fn run(dry_run: bool, force: bool, base: Option<&str>, older_than: Option<&s
3232
// Get the base branch
3333
let base_branch = if older_than.is_none() {
3434
if let Some(b) = base {
35-
b.to_string()
35+
let normalized = trim_trailing_branch_slashes(b);
36+
if normalized.is_empty() {
37+
eprintln!("{} Branch name is required", "Error:".red());
38+
std::process::exit(1);
39+
}
40+
normalized.to_string()
3641
} else {
3742
match get_default_branch(&repo) {
3843
Ok(b) => b,

src/commands/remove.rs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use colored::Colorize;
22

33
use crate::git::{discover_repo, list_worktrees, remove_worktree};
44
use crate::models::Worktree;
5+
use crate::utils::trim_trailing_branch_slashes;
56

67
pub fn run(name: Option<&str>, force: bool, yes: bool) {
78
let repo = match discover_repo() {
@@ -24,9 +25,7 @@ pub fn run(name: Option<&str>, force: bool, yes: bool) {
2425
if name.trim().is_empty() {
2526
pick_worktree_to_remove(&worktrees)
2627
} else {
27-
match worktrees.iter().find(|wt| {
28-
wt.branch == name || wt.path == name || wt.path.ends_with(&format!("/{}", name))
29-
}) {
28+
match find_worktree_by_identifier(&worktrees, name) {
3029
Some(wt) => wt.clone(),
3130
None => {
3231
eprintln!(
@@ -110,6 +109,23 @@ pub fn run(name: Option<&str>, force: bool, yes: bool) {
110109
);
111110
}
112111

112+
fn find_worktree_by_identifier<'a>(
113+
worktrees: &'a [Worktree],
114+
identifier: &str,
115+
) -> Option<&'a Worktree> {
116+
let trimmed_identifier = identifier.trim();
117+
let normalized_branch = trim_trailing_branch_slashes(trimmed_identifier);
118+
let normalized_path = trimmed_identifier.trim_end_matches('/');
119+
120+
worktrees.iter().find(|wt| {
121+
wt.path == trimmed_identifier
122+
|| wt.path.trim_end_matches('/') == normalized_path
123+
|| (!normalized_branch.is_empty()
124+
&& (wt.branch == normalized_branch
125+
|| wt.path.ends_with(&format!("/{}", normalized_branch))))
126+
})
127+
}
128+
113129
fn pick_worktree_to_remove(worktrees: &[Worktree]) -> Worktree {
114130
let removable: Vec<&Worktree> = worktrees
115131
.iter()
@@ -148,3 +164,51 @@ fn pick_worktree_to_remove(worktrees: &[Worktree]) -> Worktree {
148164
}
149165
}
150166
}
167+
168+
#[cfg(test)]
169+
mod tests {
170+
use super::find_worktree_by_identifier;
171+
use crate::models::Worktree;
172+
use chrono::DateTime;
173+
174+
fn make_worktree(path: &str, branch: &str) -> Worktree {
175+
Worktree {
176+
path: path.to_string(),
177+
branch: branch.to_string(),
178+
head: "abc123".to_string(),
179+
created_at: DateTime::from_timestamp(0, 0).unwrap(),
180+
is_dirty: false,
181+
is_locked: false,
182+
is_prunable: false,
183+
is_main: false,
184+
}
185+
}
186+
187+
#[test]
188+
fn find_worktree_by_identifier_matches_branch_with_trailing_slash() {
189+
let worktrees = vec![make_worktree(
190+
"/repo/feature/my-branch",
191+
"feature/my-branch",
192+
)];
193+
194+
let found = find_worktree_by_identifier(&worktrees, "feature/my-branch/");
195+
assert_eq!(
196+
found.map(|wt| wt.branch.as_str()),
197+
Some("feature/my-branch")
198+
);
199+
}
200+
201+
#[test]
202+
fn find_worktree_by_identifier_matches_path_with_trailing_slash() {
203+
let worktrees = vec![make_worktree(
204+
"/repo/feature/my-branch",
205+
"feature/my-branch",
206+
)];
207+
208+
let found = find_worktree_by_identifier(&worktrees, "/repo/feature/my-branch/");
209+
assert_eq!(
210+
found.map(|wt| wt.branch.as_str()),
211+
Some("feature/my-branch")
212+
);
213+
}
214+
}

src/commands/sync.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use colored::Colorize;
22

33
use crate::git::{discover_repo, get_default_branch, list_worktrees, sync_branch};
4+
use crate::utils::trim_trailing_branch_slashes;
45

56
pub fn run(branch: Option<&str>) {
67
let repo = match discover_repo() {
@@ -12,7 +13,12 @@ pub fn run(branch: Option<&str>) {
1213
};
1314

1415
let target_branch = if let Some(b) = branch {
15-
b.to_string()
16+
let normalized = trim_trailing_branch_slashes(b);
17+
if normalized.is_empty() {
18+
eprintln!("{} Branch name is required", "Error:".red());
19+
std::process::exit(1);
20+
}
21+
normalized.to_string()
1622
} else {
1723
match get_default_branch(&repo) {
1824
Ok(b) => b,

src/git/worktree_manager.rs

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::path::{Path, PathBuf};
55
use std::process::Command;
66

77
use crate::models::Worktree;
8-
use crate::utils::{discover_bare_clone, get_project_root};
8+
use crate::utils::{discover_bare_clone, get_project_root, trim_trailing_branch_slashes};
99

1010
pub const MAIN_BRANCHES: &[&str] = &["main", "master"];
1111
pub const DETACHED_HEAD: &str = "detached HEAD";
@@ -247,32 +247,36 @@ pub fn find_worktree_by_name(
247247
name: &str,
248248
) -> Result<Option<Worktree>, String> {
249249
let worktrees = list_worktrees(context)?;
250+
Ok(match_worktree_by_name(&worktrees, name).cloned())
251+
}
252+
253+
fn match_worktree_by_name<'a>(worktrees: &'a [Worktree], name: &str) -> Option<&'a Worktree> {
254+
let normalized_name = trim_trailing_branch_slashes(name);
255+
256+
if normalized_name.is_empty() {
257+
return None;
258+
}
250259

251-
// First, try exact branch name match
252-
if let Some(wt) = worktrees.iter().find(|wt| wt.branch == name) {
253-
return Ok(Some(wt.clone()));
260+
// First, try exact branch name match.
261+
if let Some(wt) = worktrees.iter().find(|wt| wt.branch == normalized_name) {
262+
return Some(wt);
254263
}
255264

256-
// Try matching by directory name
265+
// Try matching by directory name.
257266
if let Some(wt) = worktrees.iter().find(|wt| {
258267
Path::new(&wt.path)
259268
.file_name()
260269
.and_then(|n| n.to_str())
261-
.map(|n| n == name)
270+
.map(|n| n == normalized_name)
262271
.unwrap_or(false)
263272
}) {
264-
return Ok(Some(wt.clone()));
273+
return Some(wt);
265274
}
266275

267-
// Try partial branch name match (suffix matching)
268-
if let Some(wt) = worktrees
276+
// Try partial branch name match (suffix matching).
277+
worktrees
269278
.iter()
270-
.find(|wt| wt.branch.ends_with(&format!("/{}", name)))
271-
{
272-
return Ok(Some(wt.clone()));
273-
}
274-
275-
Ok(None)
279+
.find(|wt| wt.branch.ends_with(&format!("/{}", normalized_name)))
276280
}
277281

278282
struct PartialWorktree {
@@ -399,6 +403,20 @@ fn metadata_created_at(meta: &fs::Metadata) -> Option<DateTime<Utc>> {
399403
#[cfg(test)]
400404
mod tests {
401405
use super::*;
406+
use chrono::DateTime;
407+
408+
fn make_worktree(path: &str, branch: &str) -> Worktree {
409+
Worktree {
410+
path: path.to_string(),
411+
branch: branch.to_string(),
412+
head: "abc123".to_string(),
413+
created_at: DateTime::from_timestamp(0, 0).unwrap(),
414+
is_dirty: false,
415+
is_locked: false,
416+
is_prunable: false,
417+
is_main: false,
418+
}
419+
}
402420

403421
// --- parseWorktreeLines tests ---
404422

@@ -460,4 +478,32 @@ mod tests {
460478
assert!(worktrees[1].is_locked);
461479
assert!(worktrees[2].is_prunable);
462480
}
481+
482+
#[test]
483+
fn match_worktree_by_name_trims_trailing_slashes() {
484+
let worktrees = vec![
485+
make_worktree("/repo/main", "main"),
486+
make_worktree("/repo/feature/my-branch", "feature/my-branch"),
487+
];
488+
489+
let found = match_worktree_by_name(&worktrees, "feature/my-branch/");
490+
assert_eq!(
491+
found.map(|wt| wt.branch.as_str()),
492+
Some("feature/my-branch")
493+
);
494+
}
495+
496+
#[test]
497+
fn match_worktree_by_name_suffix_match_with_trailing_slash() {
498+
let worktrees = vec![make_worktree(
499+
"/repo/feature/my-branch",
500+
"feature/my-branch",
501+
)];
502+
503+
let found = match_worktree_by_name(&worktrees, "my-branch/");
504+
assert_eq!(
505+
found.map(|wt| wt.branch.as_str()),
506+
Some("feature/my-branch")
507+
);
508+
}
463509
}

src/main.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ mod git;
88
mod models;
99
mod utils;
1010

11-
use crate::utils::{is_valid_git_url, parse_duration};
11+
use crate::utils::{is_valid_git_url, parse_duration, trim_trailing_branch_slashes};
1212

1313
const VERSION: &str = env!("CARGO_PKG_VERSION");
1414

1515
fn validate_branch_name(value: &str) -> Result<String, String> {
16-
let trimmed = value.trim();
16+
let trimmed = trim_trailing_branch_slashes(value);
1717
if trimmed.is_empty() {
1818
return Err("Branch name is required".to_string());
1919
}
@@ -240,3 +240,21 @@ fn main() {
240240
}
241241
}
242242
}
243+
244+
#[cfg(test)]
245+
mod tests {
246+
use super::validate_branch_name;
247+
248+
#[test]
249+
fn validate_branch_name_trims_trailing_slashes() {
250+
assert_eq!(
251+
validate_branch_name("feature/my-branch///").unwrap(),
252+
"feature/my-branch"
253+
);
254+
}
255+
256+
#[test]
257+
fn validate_branch_name_rejects_empty_after_trimming() {
258+
assert!(validate_branch_name("///").is_err());
259+
}
260+
}

src/utils.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ pub fn extract_repo_name(git_url: &str) -> Result<String, String> {
191191
Ok(repo_name.to_string())
192192
}
193193

194+
/// Normalize branch-like user input by trimming whitespace and trailing slashes.
195+
/// Preserves internal slashes (e.g. "feature/my-branch") for nested branch names.
196+
pub fn trim_trailing_branch_slashes(value: &str) -> &str {
197+
value.trim().trim_end_matches('/')
198+
}
199+
194200
/// Normalize human-friendly duration strings to ISO 8601 format.
195201
/// Accepts formats like: 30d, 2w, 6M, 1y, 12h, 30m
196202
/// Returns ISO 8601 format: P30D, P2W, P6M, P1Y, PT12H, PT30M

0 commit comments

Comments
 (0)