Remove tparm0

Per code review, we think that tparm does nothing when there are no parameters,
and it is safe to remove it, even though this is a break from C++. This
simplifies some code.

fish-rust/src/builtins/set_color.rs

@@ -6,22 +6,13 @@ use super::shared::{
 };
 use crate::color::RgbColor;
 use crate::common::str2wcstring;
-use crate::curses::{self, tparm0, Term};
+use crate::curses::{self, Term};
 use crate::ffi::parser_t;
 use crate::output::{self, writembs_nofail, Outputter};
 use crate::wchar::{wstr, L};
 use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t};
 use crate::wutil::wgettext_fmt;
 use libc::c_int;
-use std::ffi::CString;
-
-// Helper to make curses::tparm0 more convenient.
-fn tparm(s: &Option<CString>) -> Option<CString> {
-    match s {
-        None => None,
-        Some(s) => tparm0(s),
-    }
-}
 
 #[allow(clippy::too_many_arguments)]
 fn print_modifiers(
@@ -45,7 +36,7 @@ fn print_modifiers(
         ..
     } = term;
     if bold && enter_bold_mode.is_some() {
-        writembs_nofail!(outp, tparm(enter_bold_mode));
+        writembs_nofail!(outp, enter_bold_mode);
     }
 
     if underline && enter_underline_mode.is_some() {
@@ -66,7 +57,7 @@ fn print_modifiers(
         writembs_nofail!(outp, enter_standout_mode);
     }
     if !bg.is_none() && bg.is_normal() {
-        writembs_nofail!(outp, tparm(exit_attribute_mode));
+        writembs_nofail!(outp, exit_attribute_mode);
     }
 }
 
@@ -110,7 +101,7 @@ fn print_colors(
             // If we have a background, stop it after the color
             // or it goes to the end of the line and looks ugly.
             if let Some(term) = term.as_ref() {
-                writembs_nofail!(outp, tparm(&term.exit_attribute_mode));
+                writembs_nofail!(outp, &term.exit_attribute_mode);
             }
         }
         outp.writech('\n');
@@ -241,18 +232,20 @@ pub fn set_color(
     let Some(term) = curses::term() else {
         return STATUS_CMD_ERROR;
     };
-    let Some(exit_attribute_mode) = term.exit_attribute_mode.as_ref() else {
+    let exit_attribute_mode = &term.exit_attribute_mode;
+    if exit_attribute_mode.is_none() {
         return STATUS_CMD_ERROR;
-    };
+    }
+
     let outp = &mut output::Outputter::new_buffering();
     print_modifiers(outp, &term, bold, underline, italics, dim, reverse, bg);
     if bgcolor.is_some() && bg.is_normal() {
-        writembs_nofail!(outp, tparm0(exit_attribute_mode));
+        writembs_nofail!(outp, exit_attribute_mode);
     }
 
     if !fg.is_none() {
         if fg.is_normal() || fg.is_reset() {
-            writembs_nofail!(outp, tparm0(exit_attribute_mode));
+            writembs_nofail!(outp, exit_attribute_mode);
         } else if !outp.write_color(fg, true /* is_fg */) {
             // We need to do *something* or the lack of any output messes up
             // when the cartesian product here would make "foo" disappear:

fish-rust/src/curses.rs

@@ -319,18 +319,6 @@ impl FlagCap {
 }
 
 /// Covers over tparm().
-pub fn tparm0(cap: &CStr) -> Option<CString> {
-    // Take the lock because tparm races with del_curterm, etc.
-    let _term = TERM.lock().unwrap();
-    let cap_ptr = cap.as_ptr() as *mut libc::c_char;
-    // Safety: we're trusting tparm here.
-    unsafe {
-        // Check for non-null and non-empty string.
-        assert!(!cap_ptr.is_null() && cap_ptr.read() != 0);
-        try_ptr_to_cstr(tparm(cap_ptr))
-    }
-}
-
 pub fn tparm1(cap: &CStr, param1: i32) -> Option<CString> {
     // Take the lock because tparm races with del_curterm, etc.
     let _term: std::sync::MutexGuard<Option<Arc<Term>>> = TERM.lock().unwrap();

fish-rust/src/output.rs

@@ -1,7 +1,7 @@
 // Generic output functions.
 use crate::color::RgbColor;
 use crate::common::{self, assert_is_locked, wcs2string_appending};
-use crate::curses::{self, tparm0, tparm1, Term};
+use crate::curses::{self, tparm1, Term};
 use crate::env::EnvVar;
 use crate::flog::FLOG;
 use crate::wchar::{wstr, WString, L};
@@ -131,6 +131,11 @@ fn write_color_escape(
 /// Helper to allow more convenient usage of Option<CString>.
 /// This is similar to C++ checks like `set_a_foreground && set_a_foreground[0]`
 trait CStringIsSomeNonempty {
+    /// Returns whether this string is Some and non-empty.
+    fn is_nonempty(&self) -> bool {
+        self.if_nonempty().is_some()
+    }
+
     /// Returns Some if we contain a non-empty CString.
     fn if_nonempty(&self) -> Option<&CString>;
 }
@@ -336,7 +341,7 @@ impl Outputter {
             }
         }
 
-        if term.enter_bold_mode.if_nonempty().is_some() {
+        if term.enter_bold_mode.is_nonempty() {
             if bg_set && !last_bg_set {
                 // Background color changed and is set, so we enter bold mode to make reading easier.
                 // This means bold mode is _always_ on when the background color is set.
@@ -383,10 +388,8 @@ impl Outputter {
         }
 
         // Lastly, we set bold, underline, italics, dim, and reverse modes correctly.
-        let enter_bold_mode = term.enter_bold_mode.if_nonempty();
+        if is_bold && !self.was_bold && term.enter_bold_mode.is_nonempty() && !bg_set {
-        if is_bold && !self.was_bold && enter_bold_mode.is_some() && !bg_set {
+            writembs_nofail!(self, &term.enter_bold_mode);
-            // TODO: rationalize why only this one has the tparm0 call.
-            writembs_nofail!(self, tparm0(enter_bold_mode.unwrap()));
             self.was_bold = is_bold;
         }
 
@@ -398,16 +401,16 @@ impl Outputter {
         }
         self.was_underline = is_underline;
 
-        if self.was_italics && !is_italics && term.exit_italics_mode.if_nonempty().is_some() {
+        if self.was_italics && !is_italics && term.exit_italics_mode.is_nonempty() {
             writembs_nofail!(self, &term.exit_italics_mode);
             self.was_italics = is_italics;
         }
-        if !self.was_italics && is_italics && term.enter_italics_mode.if_nonempty().is_some() {
+        if !self.was_italics && is_italics && term.enter_italics_mode.is_nonempty() {
             writembs_nofail!(self, &term.enter_italics_mode);
             self.was_italics = is_italics;
         }
 
-        if is_dim && !self.was_dim && term.enter_dim_mode.if_nonempty().is_some() {
+        if is_dim && !self.was_dim && term.enter_dim_mode.is_nonempty() {
             writembs_nofail!(self, &term.enter_dim_mode);
             self.was_dim = is_dim;
         }
@@ -415,10 +418,10 @@ impl Outputter {
 
         if is_reverse && !self.was_reverse {
             // Some terms do not have a reverse mode set, so standout mode is a fallback.
-            if term.enter_reverse_mode.if_nonempty().is_some() {
+            if term.enter_reverse_mode.is_nonempty() {
                 writembs_nofail!(self, &term.enter_reverse_mode);
                 self.was_reverse = is_reverse;
-            } else if term.enter_standout_mode.if_nonempty().is_some() {
+            } else if term.enter_standout_mode.is_nonempty() {
                 writembs_nofail!(self, &term.enter_standout_mode);
                 self.was_reverse = is_reverse;
             }