h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-05-21 19:41:14 -03:00
Code Issues Packages Projects Releases Wiki Activity

Move separate bg/fg parsing up the call stack

Browse Source 
Our highlighting module allows different highlight roles for foreground
and background.

Other users of our color-parsing logic have no need for this.

Historically we have passed an "is_background" bool and only parsed what we're
interested in.  This sort of makes sense because it's exactly what we need,
but it meant that the special behavior spread quite far.

There is no real need for spreading it; a function with the behavior "parse
a text face but honor is_background" is strictly more complex than "parse a
text face". Additionally, any performance optimization is only relevant if
the user specifies faces that won't be used, which is a very unusual case.

Let's isolate this logic in the highlighting module.
This commit is contained in:
Johannes Altmanninger
2025-04-14 13:43:03 +02:00
parent 6a8a02a549
commit 1a63956860
2 changed files with 25 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
src/highlight/highlight.rs
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, SpecifiedTextFace, TextFace, TextStyling};
use crate::text_face::{parse_text_face, TextFace};
use crate::threads::assert_is_background_thread;
use crate::tokenizer::{variable_assignment_equals_pos, PipeOrRedir};
use crate::wchar::{wstr, WString, L};
@@ -143,56 +143,49 @@ pub(crate) fn resolve_spec_uncached(
vars.get_unless_empty(get_highlight_var_name(role))
.or_else(|| vars.get_unless_empty(get_highlight_var_name(get_fallback(role))))
.or_else(|| vars.get(get_highlight_var_name(HighlightRole::normal)))
.as_ref()
.map(parse_text_face_for_highlight)
.unwrap_or_else(TextFace::default)
};
let fg_var = resolve_role(highlight.foreground);
let bg_var = resolve_role(highlight.background);
let mut result = parse_text_face_for_highlight(fg_var.as_ref(), bg_var.as_ref());
let mut face = resolve_role(highlight.foreground);
// Optimization: no need to parse again if both colors are the same.
if highlight.background != highlight.foreground {
let bg_face = resolve_role(highlight.background);
face.bg = bg_face.bg;
// In case the background role is different from the foreground one, we ignore its style
// except for reverse mode.
face.style.reverse |= bg_face.style.is_reverse();
}
// Handle modifiers.
if highlight.valid_path {
if let Some(valid_path_var) = vars.get(L!("fish_color_valid_path")) {
// Historical behavior is to not apply background.
let valid_path_face = parse_text_face_for_highlight(Some(&valid_path_var), None);
let valid_path_face = parse_text_face_for_highlight(&valid_path_var);
// Apply the foreground, except if it's normal. The intention here is likely
// to only override foreground if the valid path color has an explicit foreground.
if !valid_path_face.fg.is_normal() {
result.fg = valid_path_face.fg;
face.fg = valid_path_face.fg;
}
result.style = result.style.union(valid_path_face.style);
face.style = face.style.union(valid_path_face.style);
}
}
if highlight.force_underline {
result.style.inject_underline(true);
face.style.inject_underline(true);
}
result
face
}
}
/// Return the internal color code representing the specified color.
pub(crate) fn parse_text_face_for_highlight(
fg_var: Option<&EnvVar>,
bg_var: Option<&EnvVar>,
) -> TextFace {
let parse_var = |maybe_var: Option<&EnvVar>| {
let Some(var) = maybe_var else {
return SpecifiedTextFace {
fg: Some(Color::Normal),
bg: Some(Color::Normal),
style: TextStyling::default(),
};
};
parse_text_face(var.as_list())
};
let fg_face = parse_var(fg_var);
let bg_face = parse_var(bg_var);
let fg = fg_face.fg.unwrap_or(Color::Normal);
let bg = bg_face.bg.unwrap_or(Color::Normal);
// In case the background role is different from the foreground one, we ignore its style
// except for reverse mode.
let mut style = fg_face.style;
style.reverse |= bg_face.style.reverse;
pub(crate) fn parse_text_face_for_highlight(var: &EnvVar) -> TextFace {
let face = parse_text_face(var.as_list());
let fg = face.fg.unwrap_or(Color::Normal);
let bg = face.bg.unwrap_or(Color::Normal);
let style = face.style;
TextFace { fg, bg, style }
}

5
src/reader.rs
View File

@@ -2692,10 +2692,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) {
let mut outp = Outputter::stdoutput().borrow_mut();
if let Some(fish_color_cancel) = self.vars().get(L!("fish_color_cancel")) {
outp.set_text_face(parse_text_face_for_highlight(
Some(&fish_color_cancel),
Some(&fish_color_cancel),
));
outp.set_text_face(parse_text_face_for_highlight(&fish_color_cancel));
}
outp.write_wstr(L!("^C"));
outp.reset_text_face(true);