a73x

4fd458e4

Drop --label from patch list; patches have no labels

a73x   2026-08-09 16:50

Commit message
Drop --label from patch list; patches have no labels

patches have no labelling mechanism at all -- no `patch label`
command, no PatchLabel event, no `labels` field on PatchState. A
`--label` flag on `patch list` that always returns an empty result
would be worse than no flag: it reads as "no patches have this
label" when the concept doesn't exist yet, silently misleading
scripts and agents. Reject it as an unknown argument instead, same
as clap already does for any other unsupported flag.

`issue list --label` is unaffected. The Listable::labels() trait
default and filter_sort_paginate() plumbing stay in place (harmless,
and lets patch labelling drop in cleanly if it's ever added), but
patch.rs now always passes an empty label slice internally since
there is no user-facing flag to source one from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
src/cli.rs
Old New
@@ -342,9 +342,6 @@ pub enum PatchCmd {
342 /// Sort order: recent (default), created, alpha 342 /// Sort order: recent (default), created, alpha
343 #[arg(long, default_value = "recent")] 343 #[arg(long, default_value = "recent")]
344 sort: SortMode, 344 sort: SortMode,
345 /// Filter by label (repeatable; matches patches carrying any of the given labels)
346 #[arg(long)]
347 label: Vec<String>,
348 }, 345 },
349 /// Show patch details 346 /// Show patch details
350 Show { 347 Show {
src/lib.rs
Old New
@@ -348,14 +348,13 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
348 offset, 348 offset,
349 json, 349 json,
350 sort, 350 sort,
351 label,
352 } => { 351 } => {
353 if json { 352 if json {
354 let output = patch::list_json(repo, all, archived, sort, &label)?; 353 let output = patch::list_json(repo, all, archived, sort)?;
355 println!("{}", output); 354 println!("{}", output);
356 return Ok(()); 355 return Ok(());
357 } 356 }
358 let entries = patch::list(repo, all, archived, limit, offset, sort, &label)?; 357 let entries = patch::list(repo, all, archived, limit, offset, sort)?;
359 if entries.is_empty() { 358 if entries.is_empty() {
360 println!("No patches found."); 359 println!("No patches found.");
361 } else { 360 } else {
src/patch.rs
Old New
@@ -170,14 +170,16 @@ pub fn list(
170 limit: Option<usize>, 170 limit: Option<usize>,
171 offset: Option<usize>, 171 offset: Option<usize>,
172 sort: SortMode, 172 sort: SortMode,
173 labels: &[String],
174 ) -> Result<Vec<ListEntry>, crate::error::Error> { 173 ) -> Result<Vec<ListEntry>, crate::error::Error> {
175 let patches = if show_archived { 174 let patches = if show_archived {
176 state::list_patches_with_archived(repo)? 175 state::list_patches_with_archived(repo)?
177 } else { 176 } else {
178 state::list_patches(repo)? 177 state::list_patches(repo)?
179 }; 178 };
180 let filtered = cli::filter_sort_paginate(patches, show_closed, labels, sort, offset, limit); 179 // Patches have no labelling mechanism yet (no `patch label` command, no
180 // `labels` field on PatchState), so there is no CLI flag to plumb a
181 // label filter through here. Pass an empty slice, which is a no-op.
182 let filtered = cli::filter_sort_paginate(patches, show_closed, &[], sort, offset, limit);
181 let entries = filtered 183 let entries = filtered
182 .into_iter() 184 .into_iter()
183 .map(|patch| { 185 .map(|patch| {
@@ -188,7 +190,6 @@ pub fn list(
188 Ok(entries) 190 Ok(entries)
189 } 191 }
190 192
191 #[allow(clippy::too_many_arguments)]
192 pub fn list_to_writer( 193 pub fn list_to_writer(
193 repo: &Repository, 194 repo: &Repository,
194 show_closed: bool, 195 show_closed: bool,
@@ -196,10 +197,9 @@ pub fn list_to_writer(
196 limit: Option<usize>, 197 limit: Option<usize>,
197 offset: Option<usize>, 198 offset: Option<usize>,
198 sort: SortMode, 199 sort: SortMode,
199 labels: &[String],
200 writer: &mut dyn std::io::Write, 200 writer: &mut dyn std::io::Write,
201 ) -> Result<(), crate::error::Error> { 201 ) -> Result<(), crate::error::Error> {
202 let entries = list(repo, show_closed, show_archived, limit, offset, sort, labels)?; 202 let entries = list(repo, show_closed, show_archived, limit, offset, sort)?;
203 if entries.is_empty() { 203 if entries.is_empty() {
204 writeln!(writer, "No patches found.").ok(); 204 writeln!(writer, "No patches found.").ok();
205 return Ok(()); 205 return Ok(());
@@ -230,9 +230,8 @@ pub fn list_json(
230 show_closed: bool, 230 show_closed: bool,
231 show_archived: bool, 231 show_archived: bool,
232 sort: SortMode, 232 sort: SortMode,
233 labels: &[String],
234 ) -> Result<String, crate::error::Error> { 233 ) -> Result<String, crate::error::Error> {
235 let entries = list(repo, show_closed, show_archived, None, None, sort, labels)?; 234 let entries = list(repo, show_closed, show_archived, None, None, sort)?;
236 let patches: Vec<&PatchState> = entries.iter().map(|e| &e.patch).collect(); 235 let patches: Vec<&PatchState> = entries.iter().map(|e| &e.patch).collect();
237 Ok(serde_json::to_string_pretty(&patches)?) 236 Ok(serde_json::to_string_pretty(&patches)?)
238 } 237 }
tests/cli_test.rs
Old New
@@ -592,20 +592,28 @@ fn test_patch_list_filters_by_status() {
592 } 592 }
593 593
594 #[test] 594 #[test]
595 fn test_patch_list_label_flag_accepted() { 595 fn test_patch_list_rejects_label_flag() {
596 // Patches have no way to carry labels yet (there is no `patch label` 596 // Patches have no way to carry labels (there is no `patch label`
597 // command, unlike `issue label`), so `--label` can never match anything 597 // command, unlike `issue label`, and no `labels` field on PatchState).
598 // today. The flag must still parse -- including repeated occurrences -- 598 // A `--label` flag that always returns an empty list would be worse
599 // and correctly filter every patch out rather than erroring or being 599 // than no flag at all -- it looks like "no patches have this label"
600 // silently ignored. 600 // when really the concept doesn't exist. So `patch list` must reject
601 // `--label` outright as an unknown argument rather than silently
602 // accepting and no-op-filtering it.
601 let repo = TestRepo::new("Alice", "alice@example.com"); 603 let repo = TestRepo::new("Alice", "alice@example.com");
602 repo.patch_create("Open patch"); 604 repo.patch_create("Open patch");
603 605
604 let out = repo.run_ok(&["patch", "list"]); 606 let output = repo.run(&["patch", "list", "--label", "bug"]);
605 assert!(out.contains("Open patch")); 607 assert!(
606 608 !output.status.success(),
607 let out = repo.run_ok(&["patch", "list", "--label", "bug", "--label", "docs"]); 609 "patch list --label should be rejected"
608 assert!(out.contains("No patches found")); 610 );
611 let stderr = String::from_utf8(output.stderr).unwrap();
612 assert!(
613 stderr.contains("unexpected argument") || stderr.contains("unrecognized"),
614 "expected an unknown-argument error, got: {}",
615 stderr
616 );
609 } 617 }
610 618
611 #[test] 619 #[test]
tests/collab_test.rs
Old New
@@ -1275,7 +1275,6 @@ fn capture_patch_list(
1275 limit, 1275 limit,
1276 offset, 1276 offset,
1277 git_collab::cli::SortMode::Recent, 1277 git_collab::cli::SortMode::Recent,
1278 &[],
1279 &mut buf, 1278 &mut buf,
1280 ) 1279 )
1281 .unwrap(); 1280 .unwrap();
@@ -1550,7 +1549,7 @@ fn test_patch_list_json_output() {
1550 create_patch(&repo, &bob(), "Patch two"); 1549 create_patch(&repo, &bob(), "Patch two");
1551 1550
1552 let json_str = 1551 let json_str =
1553 git_collab::patch::list_json(&repo, false, false, git_collab::cli::SortMode::Recent, &[]) 1552 git_collab::patch::list_json(&repo, false, false, git_collab::cli::SortMode::Recent)
1554 .unwrap(); 1553 .unwrap();
1555 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap(); 1554 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
1556 let arr = value.as_array().unwrap(); 1555 let arr = value.as_array().unwrap();
tests/sort_test.rs
Old New
@@ -231,7 +231,7 @@ fn test_patch_default_sort_by_recency() {
231 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z"); 231 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z");
232 232
233 let entries = 233 let entries =
234 git_collab::patch::list(&repo, true, false, None, None, SortMode::Recent, &[]).unwrap(); 234 git_collab::patch::list(&repo, true, false, None, None, SortMode::Recent).unwrap();
235 assert_eq!(entries.len(), 2); 235 assert_eq!(entries.len(), 2);
236 assert_eq!(entries[0].patch.title, "Alpha patch"); 236 assert_eq!(entries[0].patch.title, "Alpha patch");
237 assert_eq!(entries[1].patch.title, "Beta patch"); 237 assert_eq!(entries[1].patch.title, "Beta patch");
@@ -254,7 +254,7 @@ fn test_patch_sort_by_created() {
254 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z"); 254 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z");
255 255
256 let entries = 256 let entries =
257 git_collab::patch::list(&repo, true, false, None, None, SortMode::Created, &[]).unwrap(); 257 git_collab::patch::list(&repo, true, false, None, None, SortMode::Created).unwrap();
258 assert_eq!(entries.len(), 2); 258 assert_eq!(entries.len(), 2);
259 assert_eq!(entries[0].patch.title, "Beta patch"); 259 assert_eq!(entries[0].patch.title, "Beta patch");
260 assert_eq!(entries[1].patch.title, "Alpha patch"); 260 assert_eq!(entries[1].patch.title, "Alpha patch");
@@ -269,8 +269,7 @@ fn test_patch_sort_alpha() {
269 create_patch_at(&repo, &alice(), "Apple patch", "2025-06-01T00:00:00Z"); 269 create_patch_at(&repo, &alice(), "Apple patch", "2025-06-01T00:00:00Z");
270 create_patch_at(&repo, &alice(), "Mango patch", "2025-03-01T00:00:00Z"); 270 create_patch_at(&repo, &alice(), "Mango patch", "2025-03-01T00:00:00Z");
271 271
272 let entries = 272 let entries = git_collab::patch::list(&repo, true, false, None, None, SortMode::Alpha).unwrap();
273 git_collab::patch::list(&repo, true, false, None, None, SortMode::Alpha, &[]).unwrap();
274 assert_eq!(entries.len(), 3); 273 assert_eq!(entries.len(), 3);
275 assert_eq!(entries[0].patch.title, "Apple patch"); 274 assert_eq!(entries[0].patch.title, "Apple patch");
276 assert_eq!(entries[1].patch.title, "Mango patch"); 275 assert_eq!(entries[1].patch.title, "Mango patch");