From 5b663bfe38ff02adf4b92fd78750a1ed006d6ac1 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Tue, 8 Sep 2026 12:14:28 +0300 Subject: [PATCH] Fix issues found while integrating contributor PRs --- locales/nl-NL/opencadstudio.ftl | 12 ++++----- src/entities/common.rs | 19 ++++++++----- src/io/pdf_export.rs | 47 ++++++++++++++++++++------------- src/ui/color_select.rs | 15 +++-------- src/ui/ribbon/widgets.rs | 7 +++-- 5 files changed, 52 insertions(+), 48 deletions(-) diff --git a/locales/nl-NL/opencadstudio.ftl b/locales/nl-NL/opencadstudio.ftl index 08dc9dfc3..90970dca3 100644 --- a/locales/nl-NL/opencadstudio.ftl +++ b/locales/nl-NL/opencadstudio.ftl @@ -2771,11 +2771,9 @@ annotate = .mleader = MLeader .mtext-specify-insertion-point = MTEXT Plaatsingspunt specificeren: .markup = Markering - .multileader = Multiverwijs Lijn + .multileader = Multiverwijslijn .multileader-draw-order = Tekenvolgorde multiverwijslijn: - .multiline-text = - Multiline - Tekst + .multiline-text = Multilijntekst .ordinate = Ordinaat .out = Uit .qdim-select-geometry-to-dimension-enter-when-d = QDIM Selecteer de te bematen geometrie (Enter om te voltooien): @@ -3879,9 +3877,9 @@ properties = .mode = Modus .monochrome = Monochroom .more-colors = Meer kleuren... - .multileader = Multiverwijs Lijn - .multiline = Multilijn Tekst - .multileader-style = Multiverwijs Lijnstijl + .multileader = Multiverwijslijn + .multiline = Multilijntekst + .multileader-style = Multiverwijslijnstijl .multiple-lines = Meerdere regels .n-closed = N gesloten .n-density = N-dichtheid diff --git a/src/entities/common.rs b/src/entities/common.rs index cc3ed1cc5..1d1fbee41 100644 --- a/src/entities/common.rs +++ b/src/entities/common.rs @@ -114,7 +114,11 @@ pub fn format_length(value: f64) -> String { let per_foot = 12.0 * scale; let feet = (total / per_foot).trunc(); let rem = (total - feet * per_foot) / scale; - format!("{}{:.0}'-{:.*}\"", sign, feet, prec, rem) + if feet == 0.0 { + format!("{}{:.*}\"", sign, prec, rem) + } else { + format!("{}{:.0}'-{:.*}\"", sign, feet, prec, rem) + } } 4 | 5 => { // Architectural and fractional formats use 1/64-inch resolution. @@ -142,13 +146,10 @@ pub fn format_length(value: f64) -> String { }; let unit_suffix = if ctx.lunits == 4 { "\"" } else { "" }; match feet { - Some(f) => { - if f == 0.0 && whole > 0.0 { - format!("{}{:.0}{}{}", sign, whole, frac_str, unit_suffix) - } else { - format!("{}{:.0}'-{:.0}{}{}", sign, f, whole, frac_str, unit_suffix) - } + Some(f) if f == 0.0 => { + format!("{}{:.0}{}{}", sign, whole, frac_str, unit_suffix) } + Some(f) => format!("{}{:.0}'-{:.0}{}{}", sign, f, whole, frac_str, unit_suffix), None => format!("{}{:.0}{}", sign, whole, frac_str), } } @@ -1109,6 +1110,8 @@ mod length_format_tests { #[test] fn architectural_carries_into_feet() { assert_eq!(with_units(4, 4, 11.99), "11 63/64\""); + assert_eq!(with_units(4, 4, 0.5), "0 1/2\""); + assert_eq!(with_units(4, 4, -9.25), "-9 1/4\""); assert_eq!(with_units(4, 4, 11.999), "1'-0\""); assert_eq!(with_units(4, 4, 23.999), "2'-0\""); assert_eq!(with_units(4, 4, 66.5), "5'-6 1/2\""); @@ -1118,6 +1121,8 @@ mod length_format_tests { #[test] fn engineering_carries_into_feet() { assert_eq!(with_units(3, 1, 11.94), "11.9\""); + assert_eq!(with_units(3, 1, 0.5), "0.5\""); + assert_eq!(with_units(3, 1, -9.25), "-9.3\""); assert_eq!(with_units(3, 1, 11.99), "1'-0.0\""); assert_eq!(with_units(3, 1, 23.99), "2'-0.0\""); assert_eq!(with_units(3, 2, 11.999), "1'-0.00\""); diff --git a/src/io/pdf_export.rs b/src/io/pdf_export.rs index 88911fd21..10b72da48 100644 --- a/src/io/pdf_export.rs +++ b/src/io/pdf_export.rs @@ -20,8 +20,6 @@ use printpdf::{ PdfFontHandle, PdfPage, PdfSaveOptions, Point, Polygon, PolygonRing, Pt, Rgb, TextItem, WindingOrder, }; -#[cfg(not(target_arch = "wasm32"))] -use std::io::Write; use std::path::Path; #[derive(Clone, Debug)] @@ -192,24 +190,19 @@ pub fn export_pdf_pages( write_pdf_atomically(path, &bytes) } -/// Atomically write PDF bytes to a file. Writes to a temporary file first, -/// then renames it to the target path to avoid crashes when overwriting -/// a file that is locked by another process (e.g., PDF reader). +/// Write a complete PDF beside the destination, then replace it atomically. #[cfg(not(target_arch = "wasm32"))] fn write_pdf_atomically(path: &Path, bytes: &[u8]) -> Result<(), String> { - let dir = path.parent().unwrap_or_else(|| std::path::Path::new(".")); - let file_name = path.file_name() - .and_then(|n| n.to_str()) - .ok_or_else(|| "Invalid file path".to_string())?; - let temp_path = dir.join(format!(".{}.tmp", file_name)); - - let mut file = std::fs::File::create(&temp_path) - .map_err(|e| format!("Failed to create temporary PDF file: {}", e))?; - file.write_all(bytes) - .map_err(|e| format!("Failed to write PDF data: {}", e))?; - - std::fs::rename(&temp_path, path) - .map_err(|e| format!("Failed to write PDF file: {} (the file may be open in another application)", e)) + let temp_path = super::save_temp_path(path); + if let Err(error) = std::fs::write(&temp_path, bytes) { + let _ = std::fs::remove_file(&temp_path); + return Err(format!("Failed to write PDF data: {error}")); + } + if let Err(error) = super::replace_save_file(&temp_path, path) { + let _ = std::fs::remove_file(&temp_path); + return Err(format!("Failed to replace PDF file: {error}")); + } + Ok(()) } /// Show a parented PDF save-file dialog and return the chosen path. @@ -1523,6 +1516,24 @@ fn emit_text( mod tests { use super::*; + #[test] + fn atomic_write_replaces_an_existing_pdf() { + let path = std::env::temp_dir().join(format!( + "ocs-pdf-replace-{}-{}.pdf", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::write(&path, b"old").unwrap(); + + write_pdf_atomically(&path, b"new").unwrap(); + + assert_eq!(std::fs::read(&path).unwrap(), b"new"); + std::fs::remove_file(path).unwrap(); + } + #[test] fn clip_and_scale_emit_pdf_bytes() { let w = PlotWire { diff --git a/src/ui/color_select.rs b/src/ui/color_select.rs index 82a41c6f8..bd05c1db7 100644 --- a/src/ui/color_select.rs +++ b/src/ui/color_select.rs @@ -8,14 +8,9 @@ use crate::ui::properties::acad_color_display; use crate::ui::ROW_H; use acadrust::types::Color as AcadColor; use iced::widget::{button, column, container, row, text}; -use iced::{Background, Border, Color, Element, Length, Padding, Theme}; +use iced::{Background, Border, Color, Element, Length, Theme}; use crate::t; -/// Font size for color selector buttons (matches combo_box styling). -const FONT_SZ: f32 = ROW_H * 0.42; -/// Vertical padding for color selector buttons (matches combo_box styling). -const SELECTOR_PAD_V: f32 = (ROW_H - FONT_SZ * 1.3 - 2.0) / 2.0; - /// Which "logical" entries the colour list offers besides the standard ACI /// colours. #[derive(Clone, Copy, Default)] @@ -228,12 +223,8 @@ fn color_selector_with_indicator<'a>( .align_y(iced::Center), ) .on_press(on_toggle) - .padding(Padding { - top: SELECTOR_PAD_V, - bottom: SELECTOR_PAD_V, - left: 6.0, - right: 6.0, - }) + .padding([3, 6]) + .height(ROW_H) .width(Length::Fill); if !open { diff --git a/src/ui/ribbon/widgets.rs b/src/ui/ribbon/widgets.rs index 7d9276c40..7a77c6d9d 100644 --- a/src/ui/ribbon/widgets.rs +++ b/src/ui/ribbon/widgets.rs @@ -128,14 +128,13 @@ fn measure_large_width(renderer: &iced::Renderer, label: &str) -> f32 { .0; let max_label_height = line_height * LARGE_LABEL_LINES + 0.5; let fits = |width: f32| { - ribbon_label_bounds( + let bounds = ribbon_label_bounds( renderer, label, width, advanced_text::Wrapping::Word, - ) - .height - <= max_label_height + ); + bounds.width <= width + 0.5 && bounds.height <= max_label_height }; if fits(base_inner) {