From 51cf65d7c8fcdd606b426143e041ca4d58690489 Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Tue, 22 Apr 2025 19:08:21 +0200 Subject: [PATCH] Remove e?println macros These macros are problematic because: - They panic when the output stream is closed. - They are not aware of fish's encoding of arbitrary bytes into a section of a Unicode private use area. The custom printf macros handle this. https://github.com/fish-shell/fish-shell/pull/11397 https://github.com/fish-shell/fish-shell/pull/11402 --- printf/src/arg.rs | 6 ++++++ src/bin/fish.rs | 34 ++++++++++++++++---------------- src/builtins/tests/test_tests.rs | 7 ++++--- src/flog.rs | 2 +- src/input_common.rs | 12 +++++------ src/print_help.rs | 5 +++-- src/tests/history.rs | 6 +++--- src/tests/parser.rs | 2 +- src/tests/threads.rs | 6 +++--- src/threads.rs | 2 +- src/wutil/mod.rs | 22 ++++++++++----------- 11 files changed, 56 insertions(+), 48 deletions(-) diff --git a/printf/src/arg.rs b/printf/src/arg.rs index 2abc98158..21c4c80e1 100644 --- a/printf/src/arg.rs +++ b/printf/src/arg.rs @@ -139,6 +139,12 @@ fn to_arg(self) -> Arg<'a> { } } +impl<'a> ToArg<'a> for &'a std::io::Error { + fn to_arg(self) -> Arg<'a> { + Arg::String(self.to_string()) + } +} + impl<'a> ToArg<'a> for f32 { fn to_arg(self) -> Arg<'a> { Arg::Float(self.into()) diff --git a/src/bin/fish.rs b/src/bin/fish.rs index a078a45dc..a6957d7a4 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -132,12 +132,12 @@ fn print_rusage_self() { let total_time = user_time + sys_time; let signals = rs.ru_nsignals; - eprintln!(" rusage self:"); - eprintln!(" user time: {sys_time} ms"); - eprintln!(" sys time: {user_time} ms"); - eprintln!(" total time: {total_time} ms"); - eprintln!(" max rss: {rss_kb} kb"); - eprintln!(" signals: {signals}"); + eprintf!(" rusage self:\n"); + eprintf!(" user time: %s ms\n", sys_time.to_string()); + eprintf!(" sys time: %s ms\n", user_time.to_string()); + eprintf!(" total time: %s ms\n", total_time.to_string()); + eprintf!(" max rss: %s kb\n", rss_kb.to_string()); + eprintf!(" signals: %s\n", signals.to_string()); } // Source the file config.fish in the given directory. @@ -293,7 +293,7 @@ fn fish_parse_opt(args: &mut [WString], opts: &mut FishCmdOpts) -> ControlFlow ControlFlow ControlFlow { - eprintln!( - "{}", + eprintf!( + "%ls\n", wgettext_fmt!(BUILTIN_ERR_UNKNOWN, "fish", args[w.wopt_index - 1]) ); return ControlFlow::Break(1); } ':' => { - eprintln!( - "{}", + eprintf!( + "%ls\n", wgettext_fmt!(BUILTIN_ERR_MISSING, "fish", args[w.wopt_index - 1]) ); return ControlFlow::Break(1); @@ -454,9 +453,10 @@ fn throwing_main() -> i32 { set_flog_file_fd(dbg_file.into_raw_fd()); } Err(e) => { - // TODO: should not be debug-print - eprintln!("Could not open file {:?}", debug_path); - eprintln!("{}", e); + let debug_path_string = format!("{debug_path:?}"); + // TODO: should be translated + eprintf!("Could not open file %s\n", debug_path_string); + eprintf!("%s\n", e); return 1; } }; @@ -606,7 +606,7 @@ fn throwing_main() -> i32 { wgettext!("Error reading script file '%s':"), path.to_string_lossy() ); - eprintln!("{}", e); + eprintf!("%s\n", e); } Ok(f) => { let list = &args[my_optind..]; diff --git a/src/builtins/tests/test_tests.rs b/src/builtins/tests/test_tests.rs index 43da29212..3dab58322 100644 --- a/src/builtins/tests/test_tests.rs +++ b/src/builtins/tests/test_tests.rs @@ -28,9 +28,10 @@ fn run_one_test_test_mbracket(expected: i32, lst: &[&str], bracket: bool) -> boo let result = builtin_test(&parser, &mut streams, &mut argv).builtin_status_code(); if result != expected { - eprintln!( - "expected builtin_test() to return {}, got {}", - expected, result + eprintf!( + "expected builtin_test() to return %s, got %s\n", + expected.to_string(), + result.to_string() ); } result == expected diff --git a/src/flog.rs b/src/flog.rs index 3a5251175..9a90ae600 100644 --- a/src/flog.rs +++ b/src/flog.rs @@ -225,7 +225,7 @@ fn apply_one_wildcard(wc_esc: &wstr, sense: bool) { } } if !match_found { - eprintln!("Failed to match debug category: {wc_esc}"); + eprintf!("Failed to match debug category: %ls\n", wc_esc); } } diff --git a/src/input_common.rs b/src/input_common.rs index 0fa1cf892..647430ec9 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -547,10 +547,10 @@ pub fn update_wait_on_escape_ms(vars: &EnvStack) { WAIT_ON_ESCAPE_MS.store(val.try_into().unwrap(), Ordering::Relaxed); } _ => { - eprintln!( + eprintf!( concat!( - "ignoring fish_escape_delay_ms: value '{}' ", - "is not an integer or is < 10 or >= 5000 ms" + "ignoring fish_escape_delay_ms: value '%ls' ", + "is not an integer or is < 10 or >= 5000 ms\n" ), fish_escape_delay_ms ) @@ -572,10 +572,10 @@ pub fn update_wait_on_sequence_key_ms(vars: &EnvStack) { WAIT_ON_SEQUENCE_KEY_MS.store(val.try_into().unwrap(), Ordering::Relaxed); } _ => { - eprintln!( + eprintf!( concat!( - "ignoring fish_sequence_key_delay_ms: value '{}' ", - "is not an integer or is < 10 or >= 5000 ms" + "ignoring fish_sequence_key_delay_ms: value '%ls' ", + "is not an integer or is < 10 or >= 5000 ms\n" ), sequence_key_time_ms ) diff --git a/src/print_help.rs b/src/print_help.rs index b45024f98..6f5c0ce2c 100644 --- a/src/print_help.rs +++ b/src/print_help.rs @@ -16,8 +16,9 @@ pub fn print_help(command: &str) { .spawn() .and_then(|mut c| c.wait()) { - Ok(status) if !status.success() => eprintln!("{}", HELP_ERR), - Err(e) => eprintln!("{}: {}", HELP_ERR, e), + // TODO: should be translated + Ok(status) if !status.success() => eprintf!("%s\n", HELP_ERR), + Err(e) => eprintf!("%s: %s\n", HELP_ERR, e), _ => (), } } diff --git a/src/tests/history.rs b/src/tests/history.rs index e88043b7a..42819cc4a 100644 --- a/src/tests/history.rs +++ b/src/tests/history.rs @@ -315,15 +315,15 @@ fn test_history_races() { // Remove everything from this item on let removed = list.drain(position..); for line in removed.into_iter().rev() { - println!("Item dropped from history: {line}"); + printf!("Item dropped from history: %ls\n", line); } found = true; break; } if !found { - println!( - "Line '{}' found in history, but not found in some array", + printf!( + "Line '%ls' found in history, but not found in some array\n", item.str() ); for list in &expected_lines { diff --git a/src/tests/parser.rs b/src/tests/parser.rs index c3b32f3e3..9b151f6f4 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -733,7 +733,7 @@ fn test_cancellation() { reader_push(&parser, L!(""), ReaderConfig::default()); let _pop = ScopeGuard::new((), |()| reader_pop()); - println!("Testing Ctrl-C cancellation. If this hangs, that's a bug!"); + printf!("Testing Ctrl-C cancellation. If this hangs, that's a bug!\n"); // Enable fish's signal handling here. signal_set_handlers(true); diff --git a/src/tests/threads.rs b/src/tests/threads.rs index 78961d74f..1102c5588 100644 --- a/src/tests/threads.rs +++ b/src/tests/threads.rs @@ -18,7 +18,7 @@ struct Context { let made = spawn(move || { ctx2.val.fetch_add(2, Ordering::Release); ctx2.condvar.notify_one(); - println!("condvar signalled"); + printf!("condvar signalled\n"); }); assert!(made); @@ -26,9 +26,9 @@ struct Context { let (_lock, timeout) = ctx .condvar .wait_timeout_while(lock, Duration::from_secs(5), |()| { - println!("looping with lock held"); + printf!("looping with lock held\n"); if ctx.val.load(Ordering::Acquire) != 5 { - println!("test_pthread: value did not yet reach goal"); + printf!("test_pthread: value did not yet reach goal\n"); return true; } false diff --git a/src/threads.rs b/src/threads.rs index f2c0f92ef..bcd5fdfb6 100644 --- a/src/threads.rs +++ b/src/threads.rs @@ -206,7 +206,7 @@ pub fn spawn(callback: F) -> bool { true } Err(e) => { - eprintln!("rust thread spawn failure: {e}"); + eprintf!("rust thread spawn failure: %s\n", e); false } }; diff --git a/src/wutil/mod.rs b/src/wutil/mod.rs index 818e716f8..88b72a6ba 100644 --- a/src/wutil/mod.rs +++ b/src/wutil/mod.rs @@ -98,7 +98,7 @@ pub fn perror(s: &str) { } pub fn perror_io(s: &str, e: &io::Error) { - eprintln!("{}: {}", s, e); + eprintf!("%s: %s\n", s, e); } /// Wide character version of getcwd(). @@ -350,7 +350,7 @@ mod path_cd_tests { fn relative_path() { let wd = L!("/home/user/"); let path = L!("projects"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/home/user/projects")); } @@ -358,7 +358,7 @@ fn relative_path() { fn absolute_path() { let wd = L!("/home/user/"); let path = L!("/etc"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/etc")); } @@ -366,7 +366,7 @@ fn absolute_path() { fn parent_directory() { let wd = L!("/home/user/projects/"); let path = L!("../docs"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/home/user/docs")); } @@ -374,7 +374,7 @@ fn parent_directory() { fn current_directory() { let wd = L!("/home/user/"); let path = L!("./"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/home/user")); } @@ -382,7 +382,7 @@ fn current_directory() { fn nested_parent_directory() { let wd = L!("/home/user/projects/"); let path = L!("../../"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/home")); } @@ -390,7 +390,7 @@ fn nested_parent_directory() { fn complex_path() { let wd = L!("/home/user/projects/"); let path = L!("./../other/projects/./.././../docs"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!( path_normalize_for_cd(wd, path), L!("/home/user/other/projects/./.././../docs") @@ -401,7 +401,7 @@ fn complex_path() { fn root_directory() { let wd = L!("/"); let path = L!(".."); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/..")); } @@ -409,7 +409,7 @@ fn root_directory() { fn up_to_root_directory() { let wd = L!("/foo/"); let path = L!(".."); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/")); } @@ -417,7 +417,7 @@ fn up_to_root_directory() { fn empty_path() { let wd = L!("/home/user/"); let path = L!(""); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!(path_normalize_for_cd(wd, path), L!("/home/user/")); } @@ -425,7 +425,7 @@ fn empty_path() { fn trailing_slash() { let wd = L!("/home/user/projects/"); let path = L!("docs/"); - eprintln!("({}, {})", wd, path); + eprintf!("(%ls, %ls)\n", wd, path); assert_eq!( path_normalize_for_cd(wd, path), L!("/home/user/projects/docs/")