From a3970c1661e00bdce41db12cf21059c99f830717 Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Sat, 4 Mar 2023 11:35:21 -0800
Subject: [PATCH] Improve FLOG output

Prior to this fix, the Rust FLOG output was regressed from C++, because
it put quotes around strings. However if we used Display, we would fail
to FLOG non-display types like ThreadIDs.

There is apparently no way in Rust to write a function which formats a
value preferentially using Display, falling back to Debug.

Fix this by introducing two new traits, FloggableDisplay and
FloggableDebug. FloggableDisplay is implemented for all Display types,
and FloggableDebug can be "opted into" for any Debug type:

    impl FloggableDebug for MyType {}

Both traits have a 'to_flog_str' function. FLOG brings them both into
scope, and Rust figures out which 'to_flog_str' gets called.
---
 fish-rust/src/flog.rs          | 28 +++++++++++++++++++++++++++-
 fish-rust/src/threads.rs       |  4 +++-
 fish-rust/src/topic_monitor.rs |  4 +++-
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/fish-rust/src/flog.rs b/fish-rust/src/flog.rs
index 54550f429..6f6a154c3 100644
--- a/fish-rust/src/flog.rs
+++ b/fish-rust/src/flog.rs
@@ -136,6 +136,28 @@ pub mod categories {
     );
 }
 
+/// FLOG formats values. By default we would like to use Display, and fall back to Debug.
+/// However that would require specialization. So instead we make two "separate" traits, bring them both in scope,
+/// and let Rust figure it out.
+/// Clients can opt a Debug type into Floggable by implementing FloggableDebug:
+///    impl FloggableDebug for MyType {}
+pub trait FloggableDisplay {
+    /// Return a string representation of this thing.
+    fn to_flog_str(&self) -> String;
+}
+
+impl<T: std::fmt::Display> FloggableDisplay for T {
+    fn to_flog_str(&self) -> String {
+        format!("{}", self)
+    }
+}
+
+pub trait FloggableDebug: std::fmt::Debug {
+    fn to_flog_str(&self) -> String {
+        format!("{:?}", self)
+    }
+}
+
 /// Write to our FLOG file.
 pub fn flog_impl(s: &str) {
     let fd = get_flog_file_fd().0 as RawFd;
@@ -151,9 +173,13 @@ pub fn flog_impl(s: &str) {
 macro_rules! FLOG {
     ($category:ident, $($elem:expr),+) => {
         if crate::flog::categories::$category.enabled.load(std::sync::atomic::Ordering::Relaxed) {
+            #[allow(unused_imports)]
+            use crate::flog::{FloggableDisplay, FloggableDebug};
             let mut vs = Vec::new();
             $(
-                vs.push(format!("{:?}", $elem));
+                {
+                   vs.push($elem.to_flog_str())
+                }
             )+
             // We don't use locking here so we have to append our own newline to avoid multiple writes.
             let mut v = vs.join(" ");
diff --git a/fish-rust/src/threads.rs b/fish-rust/src/threads.rs
index dc4f6419d..579eff682 100644
--- a/fish-rust/src/threads.rs
+++ b/fish-rust/src/threads.rs
@@ -1,9 +1,11 @@
 //! The rusty version of iothreads from the cpp code, to be consumed by native rust code. This isn't
 //! ported directly from the cpp code so we can use rust threads instead of using pthreads.
 
-use crate::flog::FLOG;
+use crate::flog::{FloggableDebug, FLOG};
 use std::thread::{self, ThreadId};
 
+impl FloggableDebug for ThreadId {}
+
 // We don't want to use a full-blown Lazy<T> for the cached main thread id, but we can't use
 // AtomicU64 since std::thread::ThreadId::as_u64() is a nightly-only feature (issue #67939,
 // thread_id_value). We also can't safely transmute `ThreadId` to `NonZeroU64` because there's no
diff --git a/fish-rust/src/topic_monitor.rs b/fish-rust/src/topic_monitor.rs
index 4ef936988..b4dbd291e 100644
--- a/fish-rust/src/topic_monitor.rs
+++ b/fish-rust/src/topic_monitor.rs
@@ -23,7 +23,7 @@ set. This is the real power of topics: you can wait for a sigchld signal OR a th
 use crate::fd_readable_set::fd_readable_set_t;
 use crate::fds::{self, autoclose_pipes_t};
 use crate::ffi::{self as ffi, c_int};
-use crate::flog::FLOG;
+use crate::flog::{FloggableDebug, FLOG};
 use crate::wchar::{widestrs, wstr, WString};
 use crate::wchar_ffi::wcharz;
 use nix::errno::Errno;
@@ -79,6 +79,8 @@ mod topic_monitor_ffi {
 pub use topic_monitor_ffi::{generation_list_t, topic_t};
 pub type generation_t = u64;
 
+impl FloggableDebug for topic_t {}
+
 /// A generation value which indicates the topic is not of interest.
 pub const invalid_generation: generation_t = std::u64::MAX;