set_color: allow resetting the underline style

Part 2/3 of #12495

Part of #12507
This commit is contained in:
Nahor
2026-03-03 14:09:18 -08:00
committed by Johannes Altmanninger
parent a893dd10f4
commit 4e41d142fd
6 changed files with 81 additions and 69 deletions

View File

@@ -4,7 +4,7 @@ fish ?.?.? (released ???)
Notable improvements and fixes
------------------------------
- New Spanish translations (:issue:`12489`).
- ``set_color`` is able to turn off italics, reverse mode and strikethrough individually (e.g. ``--italics=off``).
- ``set_color`` is able to turn off italics, reverse mode, strikethrough and underline individually (e.g. ``--italics=off``).
For distributors and developers
-------------------------------

View File

@@ -58,7 +58,7 @@ The following options are available:
Sets strikethrough mode. The state can be **on** / **true** (default), or **off** / **false**
**-u** or **--underline**, or **-uSTYLE** or **--underline=STYLE**
Set the underline mode; supported styles are **single** (default), **double**, **curly**, **dotted** and **dashed**.
Set the underline mode; supported styles are **single** (default), **double**, **curly**, **dotted**, **dashed** and **off**.
**--theme=THEME**
Ignored.

View File

@@ -181,7 +181,7 @@ fn resolve_spec_uncached(highlight: &HighlightSpec, vars: &dyn Environment) -> T
face.bg = bg_face.bg;
// In case the background role is different from the foreground one, we ignore its style
// except for reverse mode.
if face.style.reverse != ResettableStyle::On {
if face.style.reverse != ResettableStyle::On(()) {
face.style.reverse = bg_face.style.reverse;
}
}
@@ -202,7 +202,8 @@ fn resolve_spec_uncached(highlight: &HighlightSpec, vars: &dyn Environment) -> T
}
if highlight.force_underline {
face.style.inject_underline(UnderlineStyle::Single);
face.style
.inject_underline(ResettableStyle::On(UnderlineStyle::Single));
}
face
@@ -1318,7 +1319,7 @@ mod tests {
use crate::operation_context::{EXPANSION_LIMIT_BACKGROUND, OperationContext};
use crate::prelude::*;
use crate::tests::prelude::*;
use crate::text_face::UnderlineStyle;
use crate::text_face::{ResettableStyle, UnderlineStyle};
use libc::PATH_MAX;
// Helper to return a string whose length greatly exceeds PATH_MAX.
@@ -1854,7 +1855,7 @@ fn test_trailing_spaces_after_command() {
let face = resolver.resolve_spec(&colors[i], vars);
assert_eq!(
face.style.underline_style(),
Some(UnderlineStyle::Single),
ResettableStyle::On(UnderlineStyle::Single),
"Character at position {} of 'echo' should be underlined",
i
);
@@ -1865,7 +1866,7 @@ fn test_trailing_spaces_after_command() {
let face = resolver.resolve_spec(&colors[i], vars);
assert_eq!(
face.style.underline_style(),
None,
ResettableStyle::Off,
"Trailing space at position {} should NOT be underlined",
i
);
@@ -1922,5 +1923,7 @@ fn test_parse_text_face_for_highlight_fully_specified() {
assert_all_set(vec![L!("--reverse=off").into()]);
assert_all_set(vec![L!("--strikethrough").into()]);
assert_all_set(vec![L!("--strikethrough=off").into()]);
assert_all_set(vec![L!("--underline").into()]);
assert_all_set(vec![L!("--underline=off").into()]);
}
}

View File

@@ -390,7 +390,7 @@ fn set_text_face_internal(
// Removes all styles that are individually resettable.
let non_resettable = |mut style: TextStyling| {
style.italics = ResettableStyle::Unchanged;
style.underline_style = None;
style.underline_style = ResettableStyle::Unchanged;
style.reverse = ResettableStyle::Unchanged;
style.strikethrough = ResettableStyle::Unchanged;
style
@@ -448,14 +448,16 @@ fn set_text_face_internal(
if style.underline_style != style_writer.last().style.underline_style {
match style.underline_style {
None => {
ResettableStyle::Unchanged => {}
ResettableStyle::Off => {
if style_writer.write_command(ExitUnderlineMode) {
style_writer.last().style.underline_style = None;
style_writer.last().style.underline_style = ResettableStyle::Off;
}
}
Some(underline_style) => {
ResettableStyle::On(underline_style) => {
if style_writer.write_command(EnterUnderlineMode(underline_style)) {
style_writer.last().style.underline_style = Some(underline_style);
style_writer.last().style.underline_style =
ResettableStyle::On(underline_style);
}
}
}
@@ -481,7 +483,7 @@ fn set_text_face_internal(
ResettableStyle::Unchanged => {
return;
}
ResettableStyle::On => enter_cmd,
ResettableStyle::On(()) => enter_cmd,
ResettableStyle::Off => exit_cmd,
};
if style_writer.write_command(cmd) {
@@ -790,7 +792,7 @@ fn drop(&mut self) {
mod tests {
use fish_color::{Color, Color24};
use crate::text_face::{ResettableStyle, TextFace, TextStyling};
use crate::text_face::{TextFace, TextStyling, UnderlineStyle};
use super::{
Outputter,
@@ -889,45 +891,54 @@ fn sgr_max_length() {
#[test]
fn resettable_style_attribute() {
use ResettableStyle::{Off, On, Unchanged};
type RS = crate::text_face::ResettableStyle<()>;
type RU = crate::text_face::ResettableStyle<UnderlineStyle>;
let mut outp = Outputter::new_buffering_no_assume_normal();
let mut set_attr =
|italics: ResettableStyle, reverse: ResettableStyle, strikethrough: ResettableStyle| {
let mut style = TextStyling::unknown_style();
style.italics = italics;
style.reverse = reverse;
style.strikethrough = strikethrough;
let mut set_attr = |italics: RS, reverse: RS, strikethrough: RS, underline: RU| {
let mut style = TextStyling::unknown();
style.italics = italics;
style.reverse = reverse;
style.strikethrough = strikethrough;
style.underline_style = underline;
let face = TextFace::new(Color::None, Color::None, Color::None, style);
outp.set_text_face(face);
};
let face = TextFace::new(Color::None, Color::None, Color::None, style);
outp.set_text_face(face);
};
// `#[cfg_attr(...)]` because `#[rustfmt::skip]` triggers `error[E0658]: attributes on expressions are experimental`
// TODO: feature(stmt_expr_attributes): use #[rustfmt::skip]
#[cfg_attr(any(), rustfmt::skip)]
{
set_attr(On, Unchanged, Off);
set_attr(On, On, Unchanged);
set_attr(Unchanged, On, Unchanged);
set_attr(Unchanged, Unchanged, On);
set_attr(Off, Unchanged, On);
set_attr(Off, Off, Unchanged);
set_attr(Unchanged, Off, Unchanged);
set_attr(Unchanged, Unchanged, Off);
// There is no particular order between the different attributes.
// The main test is that for a given attribute, setting the same
// value twice (e.g. On->On) shouldn't create a new escape sequence,
// except for Unchanged which should never create a new sequence
// (which also means testing Unchanged->Unchanged is not required)
// Cherry on top: by changing what value is used first for each
// attribute, we also further exercise SGR combining, including
// an empty result.
set_attr(RS::On(()), RS::Unchanged, RS::Off, RU::On(UnderlineStyle::Curly));
set_attr(RS::On(()), RS::On(()), RS::Unchanged, RU::On(UnderlineStyle::Curly));
set_attr(RS::Unchanged, RS::On(()), RS::Unchanged, RU::Unchanged);
set_attr(RS::Unchanged, RS::Unchanged, RS::On(()), RU::Off);
set_attr(RS::Off, RS::Unchanged, RS::On(()), RU::Off);
set_attr(RS::Off, RS::Off, RS::Unchanged, RU::Unchanged);
set_attr(RS::Unchanged, RS::Off, RS::Unchanged, RU::On(UnderlineStyle::Dashed));
set_attr(RS::Unchanged, RS::Unchanged, RS::Off, RU::On(UnderlineStyle::Dotted));
}
assert_eq!(
String::from_utf8_lossy(outp.contents()),
concat!(
"\u{1b}[3;29m",
"\u{1b}[4:3;3;29m",
"\u{1b}[7m",
"",
"\u{1b}[9m",
"\u{1b}[24;9m",
"\u{1b}[23m",
"\u{1b}[27m",
"",
"\u{1b}[29m",
"\u{1b}[4:5m",
"\u{1b}[4:4;29m",
)
);
}

View File

@@ -9,14 +9,20 @@ pub(crate) trait StyleSet {
}
#[derive(Default, Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) enum ResettableStyle {
pub(crate) enum ResettableStyle<T = ()>
where
T: Copy + std::fmt::Debug + Eq,
{
#[default]
Unchanged,
Off,
On,
On(T),
}
impl StyleSet for ResettableStyle {
impl<T> StyleSet for ResettableStyle<T>
where
T: Copy + std::fmt::Debug + Eq,
{
fn union_prefer_right(self, other: Self) -> Self {
if other == Self::Unchanged {
self
@@ -43,23 +49,10 @@ pub(crate) enum UnderlineStyle {
Dashed,
}
impl StyleSet for Option<UnderlineStyle> {
fn union_prefer_right(self, other: Self) -> Self {
other.or(self)
}
fn difference_prefer_empty(self, other: Self) -> Self {
if other.is_some() {
return None;
}
self
}
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(crate) struct TextStyling {
pub(crate) bold: bool,
pub(crate) underline_style: Option<UnderlineStyle>,
pub(crate) underline_style: ResettableStyle<UnderlineStyle>,
pub(crate) italics: ResettableStyle,
pub(crate) dim: bool,
pub(crate) reverse: ResettableStyle,
@@ -70,7 +63,7 @@ impl TextStyling {
pub(crate) const fn terminal_default() -> Self {
Self {
bold: false,
underline_style: None,
underline_style: ResettableStyle::Off,
italics: ResettableStyle::Off,
dim: false,
reverse: ResettableStyle::Off,
@@ -80,7 +73,7 @@ pub(crate) const fn terminal_default() -> Self {
pub(crate) const fn unknown() -> Self {
Self {
bold: false,
underline_style: None,
underline_style: ResettableStyle::Unchanged,
italics: ResettableStyle::Unchanged,
dim: false,
reverse: ResettableStyle::Unchanged,
@@ -94,7 +87,8 @@ pub(crate) fn is_empty(&self) -> bool {
#[cfg(test)]
pub(crate) fn all_set(&self) -> bool {
(self.italics != ResettableStyle::Unchanged)
(self.underline_style != ResettableStyle::Unchanged)
&& (self.italics != ResettableStyle::Unchanged)
&& (self.reverse != ResettableStyle::Unchanged)
&& (self.strikethrough != ResettableStyle::Unchanged)
}
@@ -132,13 +126,13 @@ pub const fn is_bold(self) -> bool {
}
#[cfg(test)]
pub const fn underline_style(self) -> Option<UnderlineStyle> {
pub const fn underline_style(self) -> ResettableStyle<UnderlineStyle> {
self.underline_style
}
/// Set the given underline style.
pub fn inject_underline(&mut self, underline: UnderlineStyle) {
self.underline_style = Some(underline);
pub fn inject_underline(&mut self, underline: ResettableStyle<UnderlineStyle>) {
self.underline_style = underline;
}
/// Returns whether the text face is dim.
@@ -248,12 +242,12 @@ pub(crate) enum ParseError<'args> {
fn parse_resettable_style<'a>(w: &WGetopter<'_, 'a, '_>) -> Result<ResettableStyle, &'a wstr> {
let Some(arg) = w.woptarg else {
return Ok(ResettableStyle::On);
return Ok(ResettableStyle::On(()));
};
if (arg == "off") || (arg == "false") {
Ok(ResettableStyle::Off)
} else if (arg == "on") || (arg == "true") {
Ok(ResettableStyle::On)
Ok(ResettableStyle::On(()))
} else {
Err(arg)
}
@@ -334,19 +328,21 @@ pub(crate) fn parse_text_face_and_options<'argarray, 'args>(
}
'u' => {
let arg = w.woptarg.unwrap_or(L!("single"));
style.underline_style = Some(if arg == "single" {
UnderlineStyle::Single
style.underline_style = if arg == "single" {
ResettableStyle::On(UnderlineStyle::Single)
} else if arg == "double" {
UnderlineStyle::Double
ResettableStyle::On(UnderlineStyle::Double)
} else if arg == "curly" {
UnderlineStyle::Curly
ResettableStyle::On(UnderlineStyle::Curly)
} else if arg == "dotted" {
UnderlineStyle::Dotted
ResettableStyle::On(UnderlineStyle::Dotted)
} else if arg == "dashed" {
UnderlineStyle::Dashed
ResettableStyle::On(UnderlineStyle::Dashed)
} else if arg == "off" {
ResettableStyle::Off
} else {
return Err(UnknownUnderlineStyle(arg));
});
};
}
'c' => {
assert!(is_builtin);

View File

@@ -136,6 +136,8 @@ string escape (set_color --underline=dotted)
# CHECK: \e\[4:4m
string escape (set_color --underline=dashed)
# CHECK: \e\[4:5m
string escape (set_color --underline=off)
# CHECK: \e\[24m
string escape (set_color f00 --background=00f --underline-color=0f0 --bold --dim --italics --reverse --strikethrough --underline=curly)
# CHECK: \e\[38\;2\;255\;0\;0\;48\;2\;0\;0\;255\;58:2::0:255:0\;1\;4:3\;2\;3\;7m\e\[9m