From 2bbef53f297dcf30965ad562b370f67d22dfa084 Mon Sep 17 00:00:00 2001 From: Bishal De Date: Tue, 20 Oct 2026 20:38:21 +0530 Subject: [PATCH] craft-print, craft-ui: show printers by name, not CUPS queue name (#192) * print, ui: a printer is shown by its name, not its CUPS queue name CUPS queue names can't hold dots or spaces, so macOS adds the printer at 192.0.2.15 as the queue `_192_0_2_15`, and the Print dialog showed that underscored name. The name the user knows is the queue's description (CUPS `printer-info`). craft-print now lists queues with `lpstat -l -p -d` (same lines, plus each queue's indented details) and keeps the first `Description:` as `Printer::description`, capped at 256 characters. Windows names are already human, so they leave it empty. craft-ui's `Printer` gains `label`; the dialog shows it, or the name when it's empty, and `shown_names` adds the queue name in brackets when two printers would show the same text. Selection and jobs still use `name`. Both structs gain a field, so apps that build them by hand add it (`..Default::default()` works for craft-ui's) when they update the pin. * print, ui: an empty first description stays first, and shown names never collide parse_lpstat tracks whether a queue has had its Description line, so an empty first one isn't replaced by a later one. shown_names checks the final texts: printers whose bracketed text another label already reads show their queue names instead. --- crates/print/src/lib.rs | 48 +++++++++++++++------ crates/print/src/tests.rs | 55 ++++++++++++++++++------ crates/print/src/tests/windows_native.rs | 8 ++-- crates/ui/src/printing.rs | 43 ++++++++++++++++-- crates/ui/src/printing/tests.rs | 36 +++++++++++++++- 5 files changed, 157 insertions(+), 33 deletions(-) diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 8fbea52b..25e03ad5 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -42,7 +42,12 @@ impl std::error::Error for Error {} #[derive(Clone, Debug, PartialEq, Eq)] pub struct Printer { + /// The destination `lp -d` and the spooler know the printer by. pub name: String, + /// The printer's human name (CUPS `printer-info`), empty when the spooler gives none. A CUPS + /// queue name can't hold dots or spaces, so a printer added as `192.0.2.15` is the queue + /// `_192_0_2_15` described as `192.0.2.15`. Windows names are already human: empty there. + pub description: String, pub default: bool, /// Whether the Windows driver reports duplex support; None when unknown (CUPS). pub two_sided: Option, @@ -103,14 +108,30 @@ fn default_destination(out: &str) -> Option<&str> { out.lines().find_map(|l| l.strip_prefix("system default destination:")).map(str::trim) } -/// Parse `lpstat -p -d` output. +/// The longest description kept, in characters: a dialog shows a name, not a paragraph. +const MAX_DESCRIPTION: usize = 256; + +/// Parse `lpstat -p -d` output, or `lpstat -l -p -d`, whose indented `Description:` line under a +/// queue becomes its [`Printer::description`]. pub fn parse_lpstat(out: &str) -> Vec { let default = default_destination(out); - out.lines() - .filter_map(|l| l.strip_prefix("printer ")) - .filter_map(|l| l.split_whitespace().next()) - .map(|n| Printer { name: n.to_string(), default: default == Some(n), two_sided: None }) - .collect() + let mut printers: Vec = Vec::new(); + // Whether the last queue has had its Description line, even an empty one. + let mut described = false; + for line in out.lines() { + if let Some(name) = line.strip_prefix("printer ").and_then(|l| l.split_whitespace().next()) { + printers.push(Printer { name: name.to_string(), description: String::new(), default: default == Some(name), two_sided: None }); + described = false; + } else if line.starts_with(char::is_whitespace) && !described { + // Only a queue's own detail lines are indented; the first description wins. + let Some(description) = line.trim_start().strip_prefix("Description:") else { continue }; + if let Some(printer) = printers.last_mut() { + printer.description = description.trim().chars().take(MAX_DESCRIPTION).collect(); + described = true; + } + } + } + printers } /// Parse `lpstat -e` output: destination names, one per line. @@ -345,9 +366,9 @@ fn lpstat_with(args: &[&str]) -> std::process::Command { c } -/// `lpstat -p -d`: the spooler's queues and the default destination. +/// `lpstat -l -p -d`: the spooler's queues with their descriptions, and the default destination. pub fn lpstat_command() -> std::process::Command { - lpstat_with(&["-p", "-d"]) + lpstat_with(&["-l", "-p", "-d"]) } /// `lpstat -e`: every destination CUPS can print to, including driverless network printers no @@ -394,7 +415,7 @@ pub fn printers_parsed(queues: &str, available: Option<&str>) -> Vec { if let Some(names) = available { for name in parse_lpstat_e(names) { if !printers.iter().any(|p| p.name == name) { - printers.push(Printer { default: default == Some(name.as_str()), name, two_sided: None }); + printers.push(Printer { default: default == Some(name.as_str()), name, description: String::new(), two_sided: None }); } } } @@ -409,7 +430,10 @@ pub(crate) fn printer_list(queues: impl IntoIterator)> = queues.into_iter().filter(|(n, _)| !n.trim().is_empty()).collect(); queues.sort_by(|a, b| a.0.cmp(&b.0)); queues.dedup_by(|a, b| a.0 == b.0); - queues.into_iter().map(|(name, two_sided)| Printer { default: default == Some(name.as_str()), name, two_sided }).collect() + queues + .into_iter() + .map(|(name, two_sided)| Printer { default: default == Some(name.as_str()), name, description: String::new(), two_sided }) + .collect() } /// A printer list in the text form craft-print's former PowerShell backend printed: one per line, @@ -428,8 +452,8 @@ pub fn parse_windows_printers(out: &str) -> Vec { _ => (l, None), }; match l.strip_prefix('*') { - Some(name) => Printer { name: name.to_string(), default: true, two_sided }, - None => Printer { name: l.to_string(), default: false, two_sided }, + Some(name) => Printer { name: name.to_string(), description: String::new(), default: true, two_sided }, + None => Printer { name: l.to_string(), description: String::new(), default: false, two_sided }, } }) .collect(); diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index fd5499e9..c3675b8b 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -9,8 +9,8 @@ fn spooler_arguments_and_printer_list() { assert_eq!( parse_lpstat(out), [ - spool::Printer { name: "Office_Laser".into(), default: true, two_sided: None }, - spool::Printer { name: "Label_Writer".into(), default: false, two_sided: None } + spool::Printer { name: "Office_Laser".into(), description: String::new(), default: true, two_sided: None }, + spool::Printer { name: "Label_Writer".into(), description: String::new(), default: false, two_sided: None } ] ); assert!(parse_lpstat("lpstat: No destinations added.\nno system default destination\n").is_empty()); @@ -44,6 +44,32 @@ fn lpstat_output_is_untranslated() { assert!(envs.iter().any(|(k, v)| k == "SOFTWARE" && v.as_deref().is_some_and(|v| !v.is_empty())), "SOFTWARE missing: {envs:?}"); } +#[test] +fn a_queue_keeps_its_human_name() { + // A CUPS queue name can't hold dots, so macOS adds the printer at 192.0.2.15 as the queue + // `_192_0_2_15`. The dialog showed that underscored name; `lpstat -l` also gives the name the + // user knows, as the queue's Description. + let out = "printer _192_0_2_15 is idle. enabled since Thu Oct 1 09:00:00 2026\n\tForm mounted:\n\tDescription: 192.0.2.15\n\tAlerts: none\n\tLocation: Description: not this one\nprinter Label_Writer disabled since Thu Oct 1 09:00:00 2026 -\n\treason unknown\n\tDescription:\nprinter Office_Laser is idle.\r\n\tDescription: Office Laser, 2nd floor\r\n\tDescription: a second one is ignored\nDescription: not indented, not a detail\nsystem default destination: _192_0_2_15\n"; + assert_eq!( + parse_lpstat(out), + [ + spool::Printer { name: "_192_0_2_15".into(), description: "192.0.2.15".into(), default: true, two_sided: None }, + spool::Printer { name: "Label_Writer".into(), description: String::new(), default: false, two_sided: None }, + spool::Printer { name: "Office_Laser".into(), description: "Office Laser, 2nd floor".into(), default: false, two_sided: None }, + ], + "each queue takes its own first Description; an empty one stays empty" + ); + // A description before any queue belongs to none, and a huge one is cut to a name's length. + assert!(parse_lpstat("\tDescription: orphan\n").is_empty()); + // An empty first description is still the first: a later one doesn't replace it. + assert_eq!(parse_lpstat("printer Q is idle.\n\tDescription:\n\tDescription: Office\n")[0].description, ""); + let long = format!("printer Q is idle.\n\tDescription: {}\n", "é".repeat(10_000)); + assert_eq!(parse_lpstat(&long)[0].description.chars().count(), 256); + // The queues come from `lpstat -l`, so the descriptions are there to read. + let args: Vec<_> = spool::lpstat_command().get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + assert_eq!(args, ["-l", "-p", "-d"]); +} + #[test] fn driverless_printers_join_the_list_without_a_queue() { // A driverless network printer (IPP, discovered on the LAN) has no queue, so `lpstat -p` @@ -53,18 +79,21 @@ fn driverless_printers_join_the_list_without_a_queue() { assert_eq!( spool::printers_parsed(queues, Some("Basement_Color\nOffice_Laser\n")), [ - spool::Printer { name: "Office_Laser".into(), default: true, two_sided: None }, - spool::Printer { name: "Basement_Color".into(), default: false, two_sided: None }, + spool::Printer { name: "Office_Laser".into(), description: String::new(), default: true, two_sided: None }, + spool::Printer { name: "Basement_Color".into(), description: String::new(), default: false, two_sided: None }, ], "queues first, then the driverless destination; the queue listed twice is not doubled" ); // An old spooler without `-e` (CUPS < 1.7) keeps its queues; only the driverless names are lost. - assert_eq!(spool::printers_parsed(queues, None), [spool::Printer { name: "Office_Laser".into(), default: true, two_sided: None }]); + assert_eq!( + spool::printers_parsed(queues, None), + [spool::Printer { name: "Office_Laser".into(), description: String::new(), default: true, two_sided: None }] + ); // The default can itself be driverless (`lpoptions -d` names a destination no queue exists // for): it keeps its marker, so the dialog preselects the user's default, not the first queue. assert_eq!( spool::printers_parsed("system default destination: Basement_Color\n", Some("Basement_Color\n")), - [spool::Printer { name: "Basement_Color".into(), default: true, two_sided: None }], + [spool::Printer { name: "Basement_Color".into(), description: String::new(), default: true, two_sided: None }], "a default destination without a queue is still the default" ); assert!(spool::parse_lpstat_e("\n \n").is_empty(), "no destinations is empty, not [\"\"]"); @@ -156,16 +185,16 @@ fn windows_printers_carry_one_default_and_keep_their_names() { // Windows marks the system default with a `*` no queue name may contain; names hold spaces, // parentheses and non-ASCII letters, and the lines end with CRLF. let printers = parse_windows_printers("*Microsoft Print to PDF\r\nHP LaserJet 400 (Copy 2)\r\nКопир Canon iR\r\n\r\n*XPS via USB\r\n"); - assert_eq!(printers[0], Printer { name: "Microsoft Print to PDF".into(), default: true, two_sided: None }); - assert_eq!(printers[1], Printer { name: "HP LaserJet 400 (Copy 2)".into(), default: false, two_sided: None }); - assert_eq!(printers[2], Printer { name: "Копир Canon iR".into(), default: false, two_sided: None }); + assert_eq!(printers[0], Printer { name: "Microsoft Print to PDF".into(), description: String::new(), default: true, two_sided: None }); + assert_eq!(printers[1], Printer { name: "HP LaserJet 400 (Copy 2)".into(), description: String::new(), default: false, two_sided: None }); + assert_eq!(printers[2], Printer { name: "Копир Canon iR".into(), description: String::new(), default: false, two_sided: None }); // A spooler marking two defaults: the first still wins, so the dialog has one to preselect. - assert_eq!(printers[3], Printer { name: "XPS via USB".into(), default: false, two_sided: None }); + assert_eq!(printers[3], Printer { name: "XPS via USB".into(), description: String::new(), default: false, two_sided: None }); assert_eq!(parse_windows_printers("\r\n"), Vec::new()); let sided = parse_windows_printers("*Office PCL\t2\r\nOffice PS\t1\r\nTab\tName\t2\r\n"); - assert_eq!(sided[0], Printer { name: "Office PCL".into(), default: true, two_sided: Some(true) }); - assert_eq!(sided[1], Printer { name: "Office PS".into(), default: false, two_sided: Some(false) }); - assert_eq!(sided[2], Printer { name: "Tab\tName".into(), default: false, two_sided: Some(true) }); + assert_eq!(sided[0], Printer { name: "Office PCL".into(), description: String::new(), default: true, two_sided: Some(true) }); + assert_eq!(sided[1], Printer { name: "Office PS".into(), description: String::new(), default: false, two_sided: Some(false) }); + assert_eq!(sided[2], Printer { name: "Tab\tName".into(), description: String::new(), default: false, two_sided: Some(true) }); } /// A PPD shaped like a Fiery's: installable options, multi-line PostScript in the choices, Latin-1 diff --git a/crates/print/src/tests/windows_native.rs b/crates/print/src/tests/windows_native.rs index 157fe68b..c42bccc9 100644 --- a/crates/print/src/tests/windows_native.rs +++ b/crates/print/src/tests/windows_native.rs @@ -21,10 +21,10 @@ fn windows_printers_are_sorted_once_with_their_default() { assert_eq!( printers, [ - Printer { name: "HP LaserJet 400 (Copy 2)".into(), default: false, two_sided: Some(true) }, - Printer { name: "Zebra Label".into(), default: false, two_sided: Some(false) }, - Printer { name: r"\\print01\Office, 2nd floor".into(), default: true, two_sided: Some(true) }, - Printer { name: "Копир Canon iR".into(), default: false, two_sided: None }, + Printer { name: "HP LaserJet 400 (Copy 2)".into(), description: String::new(), default: false, two_sided: Some(true) }, + Printer { name: "Zebra Label".into(), description: String::new(), default: false, two_sided: Some(false) }, + Printer { name: r"\\print01\Office, 2nd floor".into(), description: String::new(), default: true, two_sided: Some(true) }, + Printer { name: "Копир Canon iR".into(), description: String::new(), default: false, two_sided: None }, ] ); assert!(spool::printer_list([("Office".to_string(), None)], Some("Office 2")).iter().all(|p| !p.default)); diff --git a/crates/ui/src/printing.rs b/crates/ui/src/printing.rs index 523ecfa5..30d7ad4b 100644 --- a/crates/ui/src/printing.rs +++ b/crates/ui/src/printing.rs @@ -5,10 +5,45 @@ use std::sync::mpsc::{Receiver, TryRecvError}; #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct Printer { + /// The spooler's destination: what [`Settings::printer`] and jobs name. pub name: String, + /// What the dialog shows; empty shows `name`. CUPS, for one, describes the queue + /// `_192_0_2_15` as `192.0.2.15`, the name the user gave the printer. + pub label: String, pub default: bool, } +/// The text the dialog shows for each of `printers`, in order: its label, or its name when it has +/// none. Text two printers would share is followed by the name in brackets, so neither hides the +/// other; when even that text is shared (another printer's label is `Office (Office_1)`), those +/// printers show their names, which the spooler keeps distinct. +pub fn shown_names(printers: &[Printer]) -> Vec { + let base: Vec<&str> = printers.iter().map(|p| if p.label.trim().is_empty() { p.name.as_str() } else { p.label.as_str() }).collect(); + let mut shown: Vec = printers + .iter() + .zip(&base) + .map(|(printer, text)| { + let shared = base.iter().filter(|other| *other == text).count() > 1; + if shared && *text != printer.name { format!("{text} ({})", printer.name) } else { (*text).to_owned() } + }) + .collect(); + // Each pass turns at least one shared text into its name, or stops: at most one per printer. + for _ in 0..printers.len() { + let shared: Vec = shown.iter().map(|text| shown.iter().filter(|other| *other == text).count() > 1).collect(); + let mut changed = false; + for ((text, printer), shared) in shown.iter_mut().zip(printers).zip(shared) { + if shared && *text != printer.name { + text.clone_from(&printer.name); + changed = true; + } + } + if !changed { + break; + } + } + shown +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Duplex { Off, @@ -160,13 +195,15 @@ impl State { ui.add_enabled_ui(!self.busy(), |ui| { let width = ui.available_width().clamp(32.0, 230.0); let printer_label = ui.label(labels.printer); + let shown = shown_names(&self.printers); + let selected = self.printers.iter().zip(&shown).find(|(p, _)| p.name == self.settings.printer).map(|(_, text)| text.as_str()); egui::ComboBox::from_id_salt("printer") .width(width) .wrap_mode(egui::TextWrapMode::Truncate) - .selected_text(if self.settings.printer.is_empty() { labels.select_printer } else { &self.settings.printer }) + .selected_text(if self.settings.printer.is_empty() { labels.select_printer } else { selected.unwrap_or(&self.settings.printer) }) .show_ui(ui, |ui| { - for printer in &self.printers { - ui.selectable_value(&mut self.settings.printer, printer.name.clone(), &printer.name); + for (printer, text) in self.printers.iter().zip(&shown) { + ui.selectable_value(&mut self.settings.printer, printer.name.clone(), text); } }) .response diff --git a/crates/ui/src/printing/tests.rs b/crates/ui/src/printing/tests.rs index 1fb84a8c..47f3587a 100644 --- a/crates/ui/src/printing/tests.rs +++ b/crates/ui/src/printing/tests.rs @@ -1,7 +1,41 @@ use super::*; fn queue(name: &str, default: bool) -> Printer { - Printer { name: name.into(), default } + Printer { name: name.into(), label: String::new(), default } +} + +fn labelled(name: &str, label: &str) -> Printer { + Printer { name: name.into(), label: label.into(), default: false } +} + +#[test] +fn the_dialog_shows_a_printers_label_and_keeps_its_queue_name() { + // CUPS can't put dots in a queue name: the printer added as 192.0.2.15 is `_192_0_2_15`. + let printers = [labelled("_192_0_2_15", "192.0.2.15"), queue("Plain_Queue", false), labelled("Blank", " ")]; + assert_eq!(shown_names(&printers), ["192.0.2.15", "Plain_Queue", "Blank"], "the label, else the name"); + // Two printers described alike stay apart by their queue names; a unique label stays bare. + let printers = [labelled("Office_1", "Office"), labelled("Office_2", "Office"), labelled("Lab", "Lab printer")]; + assert_eq!(shown_names(&printers), ["Office (Office_1)", "Office (Office_2)", "Lab printer"]); + // A label that is another queue's name doesn't pass for that queue. + assert_eq!(shown_names(&[queue("Office", false), labelled("Office_2", "Office")]), ["Office", "Office (Office_2)"]); + // A label that is another printer's bracketed text doesn't make two entries read alike: the + // printers whose text is still shared show their queue names. + let printers = [labelled("Office_1", "Office"), labelled("Office_2", "Office"), labelled("Office_3", "Office (Office_1)")]; + assert_eq!(shown_names(&printers), ["Office_1", "Office (Office_2)", "Office_3"]); + assert!(shown_names(&[]).is_empty()); + + // Choosing still selects the queue: jobs go to the name, not the label. + let ctx = egui::Context::default(); + let mut state = State::default(); + let discover: Discover = Box::new(|| { + let (sender, receiver) = std::sync::mpsc::channel(); + sender.send(vec![Printer { name: "_192_0_2_15".into(), label: "192.0.2.15".into(), default: true }]).unwrap(); + receiver + }); + assert!(state.refresh(&discover)); + state.poll(&ctx); + assert_eq!(state.settings.printer, "_192_0_2_15"); + assert!(state.ready()); } #[test]