Curly underlines in set_color and fish_color_*

set_color --underline=curly outputs \e[4:3m which breaks the following
terminals:
- Terminal.app interprets it as yellow background
- abduco and dvtm interpret it as green foreground
- JetBrains terminals interprets it as yellow background
- urxvt interprets it as yellow background

terminals that interpret curly as single underline:
- tmux [1]
- emacs ansi-term [2]
- emacs vterm
- GNU screen (also wrongly turns on italic mode)
- terminology (also wrongly turns on italic mode)
- Vim's :terminal

[1]: https://github.com/orgs/tmux/discussions/4477
[2]: https://lists.gnu.org/archive/html/bug-gnu-emacs/2025-04/msg01093.html

Closes #10957
This commit is contained in:
Johannes Altmanninger
2025-04-13 15:10:57 +02:00
parent ef25f1d27b
commit cc9849c279
12 changed files with 123 additions and 38 deletions

View File

@@ -65,6 +65,7 @@ Completions
Improved terminal support
^^^^^^^^^^^^^^^^^^^^^^^^^
- Support for curly underlines in `fish_color_*` variables and :doc:`set_color <cmds/set_color>` (:issue:`10957`).
- New documentation page `Terminal Compatibility <terminal-compatibility.html>`_ (also accessible via ``man fish-terminal-compatibility``) lists required and optional terminal control sequences used by fish.
Other improvements

View File

@@ -52,8 +52,8 @@ The following options are available:
**-r** or **--reverse**
Sets reverse mode.
**-u** or **--underline**
Sets underlined mode.
**-u** or **--underline**, or **-uSTYLE** or **--underline=STYLE**
Set the underline mode; supported styles are **single** (default) and **curly**.
**-h** or **--help**
Displays help about using this command.

View File

@@ -120,6 +120,10 @@ Optional Commands
- smul
- Enter underline mode.
-
* - ``\e[4:3m``
- Su
- Enter curly underline mode.
- kitty
* - ``\e[7m``
- rev
- Enter reverse video mode (swap foreground and background colors).

View File

@@ -132,6 +132,7 @@ complete -c set -n '__fish_set_is_color true false' -a --dim -x
complete -c set -n '__fish_set_is_color true false' -a --italics -x
complete -c set -n '__fish_set_is_color true true' -a --reverse -x
complete -c set -n '__fish_set_is_color true false' -a --underline -x
complete -c set -n '__fish_set_is_color true false' -a --underline=curly -x
# Locale completions
complete -c set -n '__fish_set_is_locale; and not __fish_seen_argument -s e -l erase' -x -a '(command -sq locale; and locale -a)' -d Locale

View File

@@ -4,6 +4,6 @@ complete -c set_color -s o -l bold -d 'Make font bold'
complete -c set_color -s i -l italics -d Italicise
complete -c set_color -s d -l dim -d 'Dim text'
complete -c set_color -s r -l reverse -d 'Reverse color text'
complete -c set_color -s u -l underline -d 'Underline text'
complete -c set_color -s u -l underline -d 'Underline style' -a 'single curly'
complete -c set_color -s h -l help -d 'Display help and exit'
complete -c set_color -s c -l print-colors -d 'Print a list of all accepted color names'

View File

@@ -240,7 +240,11 @@ def parse_color(color_str):
if comp == "--bold" or comp == "-o":
bold = True
elif comp == "--underline" or comp == "-u":
underline = True
underline = "single"
elif comp.startswith("--underline="):
underline = comp.stripprefix("--underline=")
elif comp.startswith("-u"): # Multiple short options like "-rbcurly" are not yet supported.
underline = comp.stripprefix("-u")
elif comp == "--italics" or comp == "-i":
italics = True
elif comp == "--dim" or comp == "-d":
@@ -295,8 +299,8 @@ def unparse_color(col):
ret += col["color"]
if col["bold"]:
ret += " --bold"
if col["underline"]:
ret += " --underline"
if col["underline"] is not None:
ret += " --underline=" + col["underline"]
if col["italics"]:
ret += " --italics"
if col["dim"]:

View File

@@ -66,6 +66,14 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -
// and we don't error for missing colors.
return Err(STATUS_INVALID_ARGS);
}
TextFaceArgsAndOptionsResult::InvalidUnderlineStyle(arg) => {
streams.err.append(wgettext_fmt!(
"%ls: invalid underline style: %ls\n",
argv[0],
arg
));
return Err(STATUS_INVALID_ARGS);
}
TextFaceArgsAndOptionsResult::UnknownOption(unknown_option_index) => {
builtin_unknown_option(
parser,

View File

@@ -28,7 +28,7 @@
};
use crate::path::{path_as_implicit_cd, path_get_cdpath, path_get_path, paths_are_same_file};
use crate::terminal::Outputter;
use crate::text_face::{parse_text_face, TextFace};
use crate::text_face::{parse_text_face, TextFace, UnderlineStyle};
use crate::threads::assert_is_background_thread;
use crate::tokenizer::{variable_assignment_equals_pos, PipeOrRedir};
use crate::wchar::{wstr, WString, L};
@@ -168,12 +168,12 @@ pub(crate) fn resolve_spec_uncached(
if !valid_path_face.fg.is_normal() {
face.fg = valid_path_face.fg;
}
face.style = face.style.union(valid_path_face.style);
face.style = face.style.union_prefer_right(valid_path_face.style);
}
}
if highlight.force_underline {
face.style.inject_underline(true);
face.style.inject_underline(UnderlineStyle::Single);
}
face

View File

@@ -3,6 +3,7 @@
use crate::future_feature_flags::{self, FeatureFlag};
use crate::highlight::HighlightColorResolver;
use crate::tests::prelude::*;
use crate::text_face::UnderlineStyle;
use crate::wchar::prelude::*;
use crate::{
env::EnvStack,
@@ -696,8 +697,9 @@ fn test_trailing_spaces_after_command() {
// Check that 'echo' is underlined
for i in 0..4 {
let face = resolver.resolve_spec(&colors[i], vars);
assert!(
face.style.is_underline(),
assert_eq!(
face.style.underline_style(),
Some(UnderlineStyle::Single),
"Character at position {} of 'echo' should be underlined",
i
);
@@ -706,8 +708,9 @@ fn test_trailing_spaces_after_command() {
// Check that trailing spaces are NOT underlined
for i in 4..text.len() {
let face = resolver.resolve_spec(&colors[i], vars);
assert!(
!face.style.is_underline(),
assert_eq!(
face.style.underline_style(),
None,
"Trailing space at position {} should NOT be underlined",
i
);

View File

@@ -5,7 +5,7 @@
use crate::future_feature_flags::{self, FeatureFlag};
use crate::global_safety::RelaxedAtomicBool;
use crate::screen::{is_dumb, only_grayscale};
use crate::text_face::{TextFace, TextStyling};
use crate::text_face::{TextFace, TextStyling, UnderlineStyle};
use crate::threads::MainThread;
use crate::wchar::prelude::*;
use crate::FLOGF;
@@ -53,6 +53,7 @@ pub(crate) enum TerminalCommand<'a> {
EnterStandoutMode,
ExitItalicsMode,
ExitUnderlineMode,
EnterCurlyUnderlineMode,
// Screen clearing
ClearScreen,
@@ -148,6 +149,7 @@ fn write(out: &mut impl Output, sequence: &'static [u8]) -> bool {
EnterStandoutMode => ti(self, b"\x1b[7m", |t| &t.enter_standout_mode),
ExitItalicsMode => ti(self, b"\x1b[23m", |t| &t.exit_italics_mode),
ExitUnderlineMode => ti(self, b"\x1b[24m", |t| &t.exit_underline_mode),
EnterCurlyUnderlineMode => write(self, b"\x1b[4:3m"),
ClearScreen => ti(self, b"\x1b[H\x1b[2J", |term| &term.clear_screen),
ClearToEndOfLine => ti(self, b"\x1b[K", |term| &term.clr_eol),
ClearToEndOfScreen => ti(self, b"\x1b[J", |term| &term.clr_eos),
@@ -497,18 +499,19 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) {
let mut last_bg_set = false;
use TerminalCommand::{
EnterBoldMode, EnterDimMode, EnterItalicsMode, EnterReverseMode, EnterStandoutMode,
EnterUnderlineMode, ExitAttributeMode, ExitItalicsMode, ExitUnderlineMode,
EnterBoldMode, EnterCurlyUnderlineMode, EnterDimMode, EnterItalicsMode,
EnterReverseMode, EnterStandoutMode, EnterUnderlineMode, ExitAttributeMode,
ExitItalicsMode, ExitUnderlineMode,
};
// Removes all styles that are individually resettable.
let non_resettable = |mut style: TextStyling| {
style.italics = false;
style.underline = false;
style.underline_style = None;
style
};
let non_resettable_attributes_to_unset =
non_resettable(self.last.style).difference(non_resettable(style));
non_resettable(self.last.style).difference_prefer_empty(non_resettable(style));
if !non_resettable_attributes_to_unset.is_empty() {
// Only way to exit non-resettable ones is a reset of all attributes.
self.reset_text_face();
@@ -577,11 +580,22 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) {
self.last.style.bold = true;
}
let was_underline = self.last.style.is_underline();
if !style.is_underline() && was_underline && self.write_command(ExitUnderlineMode) {
self.last.style.underline = false;
} else if style.is_underline() && !was_underline && self.write_command(EnterUnderlineMode) {
self.last.style.underline = true;
if style.underline_style != self.last.style.underline_style {
match style.underline_style {
None => {
if self.write_command(ExitUnderlineMode) {
self.last.style.underline_style = None;
}
}
Some(underline) => {
if self.write_command(match underline {
UnderlineStyle::Single => EnterUnderlineMode,
UnderlineStyle::Curly => EnterCurlyUnderlineMode,
}) {
self.last.style.underline_style = Some(underline);
}
}
}
}
let was_italics = self.last.style.is_italics();

View File

@@ -3,10 +3,34 @@
use crate::wchar::prelude::*;
use crate::wgetopt::{wopt, ArgType, WGetopter, WOption};
trait StyleSet {
fn union_prefer_right(self, other: Self) -> Self;
fn difference_prefer_empty(self, other: Self) -> Self;
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) enum UnderlineStyle {
Single,
Curly,
}
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(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) struct TextStyling {
pub(crate) bold: bool,
pub(crate) underline: bool,
pub(crate) underline_style: Option<UnderlineStyle>,
pub(crate) italics: bool,
pub(crate) dim: bool,
pub(crate) reverse: bool,
@@ -16,7 +40,7 @@ impl TextStyling {
pub(crate) const fn default() -> Self {
Self {
bold: false,
underline: false,
underline_style: None,
italics: false,
dim: false,
reverse: false,
@@ -25,19 +49,23 @@ pub(crate) const fn default() -> Self {
pub(crate) fn is_empty(&self) -> bool {
*self == Self::default()
}
pub(crate) fn union(self, other: Self) -> Self {
pub(crate) fn union_prefer_right(self, other: Self) -> Self {
Self {
bold: self.is_bold() || other.is_bold(),
underline: self.is_underline() || other.is_underline(),
underline_style: self
.underline_style
.union_prefer_right(other.underline_style),
italics: self.is_italics() || other.is_italics(),
dim: self.is_dim() || other.is_dim(),
reverse: self.is_reverse() || other.is_reverse(),
}
}
pub(crate) fn difference(self, other: Self) -> Self {
pub(crate) fn difference_prefer_empty(self, other: Self) -> Self {
Self {
bold: self.is_bold() && !other.is_bold(),
underline: self.is_underline() && !other.is_underline(),
underline_style: self
.underline_style
.difference_prefer_empty(other.underline_style),
italics: self.is_italics() && !other.is_italics(),
dim: self.is_dim() && !other.is_dim(),
reverse: self.is_reverse() && !other.is_reverse(),
@@ -49,14 +77,14 @@ pub const fn is_bold(self) -> bool {
self.bold
}
/// Returns whether the text face is underlined.
pub const fn is_underline(self) -> bool {
self.underline
#[cfg(test)]
pub const fn underline_style(self) -> Option<UnderlineStyle> {
self.underline_style
}
/// Set whether the text face is underline.
pub fn inject_underline(&mut self, underline: bool) {
self.underline = underline;
/// Set the given underline style.
pub fn inject_underline(&mut self, underline: UnderlineStyle) {
self.underline_style = Some(underline);
}
/// Returns whether the text face is italics.
@@ -116,6 +144,7 @@ pub(crate) fn parse_text_face(arguments: &[WString]) -> SpecifiedTextFace {
TextFaceArgsAndOptionsResult::Ok(parsed_text_faces) => parsed_text_faces,
TextFaceArgsAndOptionsResult::PrintHelp
| TextFaceArgsAndOptionsResult::InvalidArgs
| TextFaceArgsAndOptionsResult::InvalidUnderlineStyle(_)
| TextFaceArgsAndOptionsResult::UnknownOption(_) => unreachable!(),
};
assert!(!print_color_mode);
@@ -142,6 +171,7 @@ pub(crate) enum TextFaceArgsAndOptionsResult<'a> {
Ok(TextFaceArgsAndOptions<'a>),
PrintHelp,
InvalidArgs,
InvalidUnderlineStyle(&'a wstr),
UnknownOption(usize),
}
@@ -150,12 +180,12 @@ pub(crate) fn parse_text_face_and_options<'a>(
is_builtin: bool,
) -> TextFaceArgsAndOptionsResult<'a> {
let builtin_extra_args = if is_builtin { 0 } else { "hc".len() };
let short_options = L!(":b:oidruch");
let short_options = L!(":b:oidru::ch");
let short_options = &short_options[..short_options.len() - builtin_extra_args];
let long_options: &[WOption] = &[
wopt(L!("background"), ArgType::RequiredArgument, 'b'),
wopt(L!("bold"), ArgType::NoArgument, 'o'),
wopt(L!("underline"), ArgType::NoArgument, 'u'),
wopt(L!("underline"), ArgType::OptionalArgument, 'u'),
wopt(L!("italics"), ArgType::NoArgument, 'i'),
wopt(L!("dim"), ArgType::NoArgument, 'd'),
wopt(L!("reverse"), ArgType::NoArgument, 'r'),
@@ -184,7 +214,16 @@ pub(crate) fn parse_text_face_and_options<'a>(
'i' => style.italics = true,
'd' => style.dim = true,
'r' => style.reverse = true,
'u' => style.underline = true,
'u' => {
let arg = w.woptarg.unwrap_or(L!("single"));
if arg == "single" {
style.underline_style = Some(UnderlineStyle::Single);
} else if arg == "curly" {
style.underline_style = Some(UnderlineStyle::Curly);
} else if is_builtin {
return TextFaceArgsAndOptionsResult::InvalidUnderlineStyle(arg);
}
}
'c' => print_color_mode = true,
':' => {
if is_builtin {

View File

@@ -17,3 +17,14 @@ string escape (set_color --bold red --background=normal)
# CHECK: \e\[m
string escape (set_color --bold red --background=blue)
# CHECK: \e\[1m\e\[31m\e\[44m
string escape (set_color --underline=curly)
# CHECK: \e\[4:3m
string escape (set_color -ucurly)
# CHECK: \e\[4:3m
string escape (set_color -u)
# CHECK: \e\[4m
set_color --underline=asdf
# CHECKERR: set_color: invalid underline style: asdf
set_color -ushort
# CHECKERR: set_color: invalid underline style: short